Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fde95b9266 | ||
|
|
7ae5f3a541 | ||
|
|
22d0fdd251 | ||
|
|
a8d2087b4a | ||
|
|
61c0d73cd1 | ||
|
|
6a0d7bbef4 | ||
|
|
29d96c8946 | ||
|
|
8d8d2d8f81 | ||
|
|
0d8a2c2b1d | ||
|
|
11d1d2e99f |
@@ -0,0 +1,267 @@
|
||||
"""Immutable canonical repository root for cross-repository namespaces (#706).
|
||||
|
||||
The Gitea-Tools MCP server historically derived the ``canonical_repo_root`` from
|
||||
the *install checkout* the server script lives in
|
||||
(``PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))``). A namespace
|
||||
that runs the same server script against an *external* repository (e.g.
|
||||
``eagenda-author`` targeting ``eAgenda``) then failed every mutation: the
|
||||
branches-only / worktree-membership guards (#274) compared the task workspace
|
||||
against the Gitea-Tools ``.git`` directory, which it can never belong to.
|
||||
|
||||
This module separates two distinct concepts:
|
||||
|
||||
* the immutable code/install root (``PROJECT_ROOT``) — where the server lives, and
|
||||
* the namespace-scoped **canonical repository root** — the working root of the
|
||||
repository whose issues/PRs the namespace mutates.
|
||||
|
||||
The canonical repository root is configured per namespace (profile field or an
|
||||
environment variable, typically set alongside the namespace ``cwd`` in the MCP
|
||||
config). It is validated (existence, git identity, git common-directory
|
||||
membership) and pinned immutably into the session context so a later call cannot
|
||||
forge or swap it. When *no* binding is configured the single-repo default is
|
||||
preserved unchanged: the canonical root is derived from the process checkout.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
from typing import Mapping
|
||||
|
||||
import remote_repo_guard
|
||||
|
||||
# Namespace-scoped override, typically exported next to the server ``cwd`` in the
|
||||
# MCP config for a cross-repository namespace.
|
||||
CANONICAL_ROOT_ENV = "GITEA_CANONICAL_REPOSITORY_ROOT"
|
||||
|
||||
# Candidate git remote names probed when deriving repository identity.
|
||||
_IDENTITY_REMOTE_CANDIDATES = ("prgs", "origin", "dadeschools", "mdcps")
|
||||
|
||||
|
||||
def configured_canonical_root(
|
||||
profile: Mapping | None,
|
||||
env: Mapping | None,
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""Return ``(value, source)`` for the declared canonical repository root.
|
||||
|
||||
Precedence: the ``GITEA_CANONICAL_REPOSITORY_ROOT`` environment variable
|
||||
(namespace-scoped) overrides the profile ``canonical_repository_root``
|
||||
field. Blank values are treated as unset. Returns ``(None, None)`` when no
|
||||
binding is declared (the single-repo default).
|
||||
"""
|
||||
env_map = env if env is not None else os.environ
|
||||
env_val = (env_map.get(CANONICAL_ROOT_ENV) or "").strip()
|
||||
if env_val:
|
||||
return env_val, f"{CANONICAL_ROOT_ENV} environment variable"
|
||||
if profile:
|
||||
prof_val = (profile.get("canonical_repository_root") or "").strip()
|
||||
if prof_val:
|
||||
return prof_val, "profile canonical_repository_root"
|
||||
return None, None
|
||||
|
||||
|
||||
def resolve_repo_toplevel(path: str) -> str | None:
|
||||
"""Realpath of the git working-tree top level for *path*, or None."""
|
||||
text = (path or "").strip()
|
||||
if not text:
|
||||
return None
|
||||
try:
|
||||
res = subprocess.run(
|
||||
["git", "-C", text, "rev-parse", "--show-toplevel"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
top = (res.stdout or "").strip()
|
||||
return os.path.realpath(top) if top else None
|
||||
|
||||
|
||||
def repository_identity_slug(path: str, *, remote: str | None = None) -> str | None:
|
||||
"""``owner/repository`` derived from a git remote configured at *path*.
|
||||
|
||||
Tries the caller-named remote first, then a small set of known remote names,
|
||||
then whatever remote the repository actually has. Returns None when no remote
|
||||
URL is parseable (identity cannot be proven).
|
||||
"""
|
||||
text = (path or "").strip()
|
||||
if not text:
|
||||
return None
|
||||
|
||||
ordered: list[str] = []
|
||||
for name in (remote, *_IDENTITY_REMOTE_CANDIDATES):
|
||||
clean = (name or "").strip()
|
||||
if clean and clean not in ordered:
|
||||
ordered.append(clean)
|
||||
|
||||
try:
|
||||
listed = subprocess.run(
|
||||
["git", "-C", text, "remote"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
).stdout.split()
|
||||
except Exception:
|
||||
listed = []
|
||||
for name in listed:
|
||||
if name and name not in ordered:
|
||||
ordered.append(name)
|
||||
|
||||
for name in ordered:
|
||||
try:
|
||||
url = subprocess.run(
|
||||
["git", "-C", text, "remote", "get-url", name],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
).stdout.strip()
|
||||
except Exception:
|
||||
continue
|
||||
parsed = remote_repo_guard.parse_org_repo_from_remote_url(url)
|
||||
if parsed:
|
||||
return f"{parsed[0]}/{parsed[1]}"
|
||||
return None
|
||||
|
||||
|
||||
def assess_canonical_repository_root(
|
||||
*,
|
||||
configured_value: str | None,
|
||||
source: str | None,
|
||||
expected_slug: str | None,
|
||||
process_project_root: str,
|
||||
remote: str | None = None,
|
||||
require_binding: bool = False,
|
||||
) -> dict:
|
||||
"""Validate the canonical repository root binding, failing closed on forgery.
|
||||
|
||||
Returns a dict with ``proven`` / ``block`` / ``reasons`` plus the resolved
|
||||
``canonical_repo_root`` (the value downstream guards must use),
|
||||
``configured`` (whether a cross-repo binding was declared),
|
||||
``resolved_slug`` and ``source``.
|
||||
|
||||
Without a configured binding the single-repo default is preserved: the
|
||||
canonical root is derived from *process_project_root* and never blocks
|
||||
(unless *require_binding* explicitly demands one).
|
||||
|
||||
With a configured binding the path must exist, be a git repository, and —
|
||||
when *expected_slug* is known — carry a matching repository identity. A
|
||||
mismatched or (when *require_binding*) unprovable identity is a forged or
|
||||
conflicting binding and fails closed.
|
||||
"""
|
||||
process_root = os.path.realpath(process_project_root)
|
||||
declared = (configured_value or "").strip()
|
||||
|
||||
if not declared:
|
||||
if require_binding:
|
||||
return _assessment(
|
||||
proven=False,
|
||||
reasons=[
|
||||
"no canonical_repository_root configured for a cross-repository "
|
||||
f"namespace; set {CANONICAL_ROOT_ENV} or the profile "
|
||||
"canonical_repository_root field (fail closed)"
|
||||
],
|
||||
configured=False,
|
||||
canonical_repo_root=process_root,
|
||||
resolved_slug=None,
|
||||
source=None,
|
||||
)
|
||||
# Single-repo default: canonical root follows the install checkout.
|
||||
derived = resolve_repo_toplevel(process_root) or process_root
|
||||
return _assessment(
|
||||
proven=True,
|
||||
reasons=[],
|
||||
configured=False,
|
||||
canonical_repo_root=derived,
|
||||
resolved_slug=None,
|
||||
source=None,
|
||||
)
|
||||
|
||||
real = os.path.realpath(os.path.abspath(declared))
|
||||
if not os.path.isdir(real):
|
||||
return _assessment(
|
||||
proven=False,
|
||||
reasons=[
|
||||
f"configured canonical repository root '{real}' does not exist "
|
||||
"or is not a directory (fail closed)"
|
||||
],
|
||||
configured=True,
|
||||
canonical_repo_root=real,
|
||||
resolved_slug=None,
|
||||
source=source,
|
||||
)
|
||||
|
||||
toplevel = resolve_repo_toplevel(real)
|
||||
if not toplevel:
|
||||
return _assessment(
|
||||
proven=False,
|
||||
reasons=[
|
||||
f"configured canonical repository root '{real}' is not a git "
|
||||
"repository (fail closed)"
|
||||
],
|
||||
configured=True,
|
||||
canonical_repo_root=real,
|
||||
resolved_slug=None,
|
||||
source=source,
|
||||
)
|
||||
|
||||
resolved_slug = repository_identity_slug(toplevel, remote=remote)
|
||||
reasons: list[str] = []
|
||||
expected = (expected_slug or "").strip() or None
|
||||
if expected:
|
||||
if resolved_slug and resolved_slug.lower() != expected.lower():
|
||||
reasons.append(
|
||||
f"canonical repository root identity mismatch: '{toplevel}' resolves "
|
||||
f"to repository '{resolved_slug}' but the session is authorized for "
|
||||
f"'{expected}' (forged or conflicting binding, fail closed)"
|
||||
)
|
||||
elif not resolved_slug and require_binding:
|
||||
reasons.append(
|
||||
f"canonical repository root '{toplevel}' has no resolvable git "
|
||||
f"remote identity to confirm authorization for '{expected}' "
|
||||
"(fail closed)"
|
||||
)
|
||||
|
||||
return _assessment(
|
||||
proven=not reasons,
|
||||
reasons=reasons,
|
||||
configured=True,
|
||||
canonical_repo_root=toplevel,
|
||||
resolved_slug=resolved_slug,
|
||||
source=source,
|
||||
)
|
||||
|
||||
|
||||
def format_canonical_repository_root_error(assessment: Mapping) -> str:
|
||||
"""Single RuntimeError message for MCP preflight gates."""
|
||||
root = assessment.get("canonical_repo_root") or "(unknown)"
|
||||
source = assessment.get("source") or "(unconfigured)"
|
||||
reasons = "; ".join(
|
||||
assessment.get("reasons") or ["unknown canonical repository root violation"]
|
||||
)
|
||||
return (
|
||||
f"Canonical repository root guard (#706): {reasons}. "
|
||||
f"binding source: {source}; canonical repository root: {root}. "
|
||||
"Configure a valid canonical_repository_root for the target repository "
|
||||
"and relaunch; do not point it at the Gitea-Tools install checkout."
|
||||
)
|
||||
|
||||
|
||||
def _assessment(
|
||||
*,
|
||||
proven: bool,
|
||||
reasons: list[str],
|
||||
configured: bool,
|
||||
canonical_repo_root: str,
|
||||
resolved_slug: str | None,
|
||||
source: str | None,
|
||||
) -> dict:
|
||||
return {
|
||||
"proven": proven,
|
||||
"block": not proven,
|
||||
"reasons": list(reasons),
|
||||
"configured": configured,
|
||||
"canonical_repo_root": canonical_repo_root,
|
||||
"resolved_slug": resolved_slug,
|
||||
"source": source,
|
||||
}
|
||||
@@ -802,6 +802,10 @@ def get_profile():
|
||||
# environment variable must never widen or forge the set of
|
||||
# repositories a session may bind to.
|
||||
"allowed_repositories": _json_list("allowed_repositories"),
|
||||
# #706 cross-repository canonical root binding. Config-sourced here (the
|
||||
# namespace-scoped GITEA_CANONICAL_REPOSITORY_ROOT env override is applied
|
||||
# by canonical_repository_root.configured_canonical_root, not widened here).
|
||||
"canonical_repository_root": jp.get("canonical_repository_root") or None,
|
||||
"audit_label": audit_label,
|
||||
"token_source_name": token_source,
|
||||
"auth_source_type": auth_type,
|
||||
|
||||
@@ -502,6 +502,30 @@ def _validate_allowed_repositories(name, raw):
|
||||
)
|
||||
|
||||
|
||||
def _validate_canonical_repository_root(name, raw):
|
||||
"""Validate the optional per-profile canonical repository root (#706).
|
||||
|
||||
``canonical_repository_root`` binds a cross-repository namespace to the
|
||||
working root of its target repository (separate from the immutable
|
||||
Gitea-Tools install checkout). It is an absolute filesystem path; existence
|
||||
and git identity are validated at bind time by the runtime guard, not here
|
||||
(config validation stays filesystem-independent). Absent means the
|
||||
single-repo default and is allowed.
|
||||
"""
|
||||
if raw is None:
|
||||
return
|
||||
if not isinstance(raw, str) or not raw.strip():
|
||||
raise ConfigError(
|
||||
f"profile '{name}' canonical_repository_root must be a non-empty "
|
||||
"absolute path string to the target repository working root"
|
||||
)
|
||||
if not os.path.isabs(raw.strip()):
|
||||
raise ConfigError(
|
||||
f"profile '{name}' canonical_repository_root {raw!r} must be an "
|
||||
"absolute path"
|
||||
)
|
||||
|
||||
|
||||
def _reject_inline_secrets(kind, name, obj):
|
||||
for key in _INLINE_SECRET_KEYS:
|
||||
if key in obj:
|
||||
@@ -577,6 +601,9 @@ def _load_v2_contexts(data, path):
|
||||
if not isinstance(allowed, list) or not isinstance(forbidden, list):
|
||||
raise ConfigError(f"profile '{name}' operation fields must be lists")
|
||||
_validate_allowed_repositories(name, raw.get("allowed_repositories"))
|
||||
_validate_canonical_repository_root(
|
||||
name, raw.get("canonical_repository_root")
|
||||
)
|
||||
allowed_n = {_normalize_op("gitea", op, name) for op in allowed}
|
||||
forbidden_n = {_normalize_op("gitea", op, name) for op in forbidden}
|
||||
# Reviewer-identity deadlock rule (#100/#103) applies here unchanged.
|
||||
|
||||
+436
-28
@@ -194,6 +194,7 @@ MERGER_WORKTREE_ENV = "GITEA_MERGER_WORKTREE"
|
||||
RECONCILER_WORKTREE_ENV = "GITEA_RECONCILER_WORKTREE"
|
||||
|
||||
import namespace_workspace_binding as nwb # noqa: E402
|
||||
import canonical_repository_root as crr # noqa: E402 # #706 cross-repo canonical root
|
||||
import mcp_namespace_health # noqa: E402
|
||||
import stale_binding_recovery # noqa: E402
|
||||
|
||||
@@ -376,6 +377,22 @@ def _assess_stale_active_binding(auto_recover: bool = False) -> dict:
|
||||
return report
|
||||
|
||||
|
||||
def _configured_canonical_root() -> tuple[str | None, str | None]:
|
||||
"""Declared cross-repository canonical root for the active namespace (#706).
|
||||
|
||||
Returns ``(value, source)`` from the profile/env binding, or ``(None, None)``
|
||||
for the single-repo default. The raw declared value is threaded into
|
||||
workspace resolution so the branches-only / membership guards evaluate
|
||||
against the configured target repository; the strict identity/existence
|
||||
checks run separately in :func:`_enforce_canonical_repository_root`.
|
||||
"""
|
||||
try:
|
||||
profile = get_profile()
|
||||
except Exception:
|
||||
profile = {}
|
||||
return crr.configured_canonical_root(profile, os.environ)
|
||||
|
||||
|
||||
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
|
||||
"""Resolve the namespace-scoped workspace root inspected by pre-flight guards.
|
||||
|
||||
@@ -399,8 +416,9 @@ def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
|
||||
|
||||
|
||||
def _resolve_namespace_mutation_context(worktree_path: str | None = None) -> dict:
|
||||
"""Canonical namespace workspace + repository root for guards (#460/#510)."""
|
||||
"""Canonical namespace workspace + repository root for guards (#460/#510/#706)."""
|
||||
role = _effective_workspace_role()
|
||||
configured_root, _source = _configured_canonical_root()
|
||||
return nwb.resolve_namespace_mutation_context(
|
||||
role_kind=role,
|
||||
worktree_path=worktree_path,
|
||||
@@ -409,6 +427,7 @@ def _resolve_namespace_mutation_context(worktree_path: str | None = None) -> dic
|
||||
_reviewer_session_worktree() if role in {"reviewer", "merger"} else None
|
||||
),
|
||||
profile_name=get_profile().get("profile_name"),
|
||||
configured_canonical_root=configured_root,
|
||||
)
|
||||
|
||||
|
||||
@@ -721,6 +740,54 @@ def record_preflight_check(
|
||||
_preflight_resolved_task = resolved_task
|
||||
|
||||
|
||||
def _enforce_canonical_repository_root(
|
||||
worktree_path: str | None = None,
|
||||
*,
|
||||
remote: str | None = None,
|
||||
) -> None:
|
||||
"""#706: validate the immutable cross-repository canonical root binding.
|
||||
|
||||
A no-op for the single-repo default (no configured binding). When a
|
||||
namespace declares a ``canonical_repository_root`` (profile/env), the
|
||||
configured target repository must exist, be a git repository, and carry a
|
||||
repository identity matching the session's authorized repository. Missing,
|
||||
conflicting, or forged bindings fail closed, and the validated root is
|
||||
checked against the immutable session pin so it cannot be swapped mid
|
||||
session.
|
||||
"""
|
||||
configured_value, source = _configured_canonical_root()
|
||||
if not configured_value:
|
||||
return
|
||||
|
||||
bound = session_ctx.get_session_context() or {}
|
||||
expected_slug = session_ctx.format_repository_slug(
|
||||
bound.get("org"), bound.get("repository")
|
||||
)
|
||||
assessment = crr.assess_canonical_repository_root(
|
||||
configured_value=configured_value,
|
||||
source=source,
|
||||
expected_slug=expected_slug,
|
||||
process_project_root=PROJECT_ROOT,
|
||||
remote=remote,
|
||||
require_binding=True,
|
||||
)
|
||||
if assessment["block"]:
|
||||
raise RuntimeError(
|
||||
crr.format_canonical_repository_root_error(assessment)
|
||||
)
|
||||
|
||||
drift = session_ctx.assess_session_context(
|
||||
profile_name=get_profile().get("profile_name"),
|
||||
remote=remote,
|
||||
canonical_repository_root=assessment["canonical_repo_root"],
|
||||
)
|
||||
if drift["block"]:
|
||||
raise RuntimeError(
|
||||
"Canonical repository root guard (#706): "
|
||||
+ "; ".join(drift["reasons"])
|
||||
)
|
||||
|
||||
|
||||
def _enforce_branches_only_author_mutation(worktree_path: str | None = None) -> None:
|
||||
"""#274: author file/branch mutations must run from a branches/ worktree.
|
||||
|
||||
@@ -767,6 +834,15 @@ def _anti_stomp_in_test_mode() -> bool:
|
||||
return not bool(os.environ.get("GITEA_TEST_FORCE_ANTI_STOMP"))
|
||||
|
||||
|
||||
# Mutation tasks that carry an explicit cross-repository *target* contract and
|
||||
# must NOT auto-resolve the workspace repository for omitted coordinates. Only
|
||||
# ``delete_branch`` (#733) qualifies: a destructive raw-branch deletion names
|
||||
# its org/repo explicitly and fails closed otherwise. Every other mutation is
|
||||
# session-bound to the single workspace-aligned repository and resolves it from
|
||||
# the trusted git remote (mirroring :func:`_resolve`).
|
||||
_EXPLICIT_TARGET_MUTATION_TASKS = frozenset({"delete_branch"})
|
||||
|
||||
|
||||
def _run_anti_stomp_preflight(
|
||||
task: str | None,
|
||||
*,
|
||||
@@ -832,21 +908,56 @@ def _run_anti_stomp_preflight(
|
||||
local_remote_url = None
|
||||
org_explicit = org is not None
|
||||
repo_explicit = repo is not None
|
||||
filled_org = False
|
||||
filled_repo = False
|
||||
if remote:
|
||||
try:
|
||||
local_remote_url = _local_git_remote_url(remote)
|
||||
except Exception:
|
||||
local_remote_url = None
|
||||
if resolved_org is None or resolved_repo is None:
|
||||
try:
|
||||
rem = _effective_remote(remote)
|
||||
if rem in REMOTES:
|
||||
if resolved_org is None:
|
||||
resolved_org = REMOTES[rem]["org"]
|
||||
if resolved_repo is None:
|
||||
resolved_repo = REMOTES[rem]["repo"]
|
||||
except Exception:
|
||||
pass
|
||||
# Session-bound mutations act on the single workspace-aligned
|
||||
# repository, so prefer the trusted git-remote-derived identity for
|
||||
# omitted coordinates — exactly as :func:`_resolve` does — instead
|
||||
# of the remote-wide ``REMOTES`` default *target*. A bare ``prgs``
|
||||
# default resolves to ``Timesheet`` and false-positived the #530
|
||||
# repo guard against a ``Gitea-Tools`` checkout, blocking native
|
||||
# issue/PR/review/merge/lock/cleanup mutations at preflight while
|
||||
# the actual resolve stage (``_resolve``) would have targeted the
|
||||
# workspace repo correctly. Workspace-filled sides are intentional
|
||||
# alignment and are marked explicit so the guard accepts them.
|
||||
# ``delete_branch`` keeps its explicit cross-repo target contract
|
||||
# (#733) and is excluded, so it still fails closed on omitted
|
||||
# coordinates. ``REMOTES`` stays a best-effort last resort only when
|
||||
# no workspace identity can be derived (preserves the dadeschools
|
||||
# default-target behavior and the existing unit suite).
|
||||
workspace_parts = None
|
||||
if task not in _EXPLICIT_TARGET_MUTATION_TASKS:
|
||||
try:
|
||||
workspace_parts = (
|
||||
remote_repo_guard.parse_org_repo_from_remote_url(
|
||||
local_remote_url
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
workspace_parts = None
|
||||
if workspace_parts:
|
||||
if resolved_org is None:
|
||||
resolved_org = workspace_parts[0]
|
||||
filled_org = True
|
||||
if resolved_repo is None:
|
||||
resolved_repo = workspace_parts[1]
|
||||
filled_repo = True
|
||||
if resolved_org is None or resolved_repo is None:
|
||||
try:
|
||||
rem = _effective_remote(remote)
|
||||
if rem in REMOTES:
|
||||
if resolved_org is None:
|
||||
resolved_org = REMOTES[rem]["org"]
|
||||
if resolved_repo is None:
|
||||
resolved_repo = REMOTES[rem]["repo"]
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
parity = _current_master_parity()
|
||||
startup_head = parity.get("startup_head")
|
||||
@@ -900,8 +1011,10 @@ def _run_anti_stomp_preflight(
|
||||
resolved_org=resolved_org,
|
||||
resolved_repo=resolved_repo,
|
||||
local_remote_url=local_remote_url,
|
||||
org_explicit=org_explicit,
|
||||
repo_explicit=repo_explicit,
|
||||
# Workspace-filled sides are intentional alignment for #530 (same
|
||||
# contract as :func:`_resolve`), so treat them as explicit.
|
||||
org_explicit=org_explicit or filled_org,
|
||||
repo_explicit=repo_explicit or filled_repo,
|
||||
check_repo=check_repo,
|
||||
profile_name=profile_name,
|
||||
profile_role=profile_role,
|
||||
@@ -1069,6 +1182,7 @@ def verify_preflight_purity(
|
||||
)
|
||||
|
||||
# Historical path: root + branches after purity-order when dirty paths live.
|
||||
_enforce_canonical_repository_root(worktree_path, remote=remote)
|
||||
_enforce_root_checkout_guard(worktree_path)
|
||||
_enforce_branches_only_author_mutation(worktree_path)
|
||||
_enforce_issue_scope_guard(
|
||||
@@ -1102,6 +1216,7 @@ def verify_preflight_purity(
|
||||
# #683: under pytest unit isolation, FORCE_PRODUCTION_GUARDS still runs
|
||||
# production root + branches + issue scope (no silent no-op of guards).
|
||||
if production_active:
|
||||
_enforce_canonical_repository_root(worktree_path, remote=remote)
|
||||
_enforce_root_checkout_guard(worktree_path)
|
||||
_enforce_branches_only_author_mutation(worktree_path)
|
||||
_enforce_issue_scope_guard(
|
||||
@@ -1560,7 +1675,43 @@ def _trusted_session_repository(
|
||||
"repository": None,
|
||||
"reasons": [str(exc)],
|
||||
}
|
||||
slug = _workspace_repository_slug(remote)
|
||||
# #706 F1: when a cross-repository canonical root is configured, the session
|
||||
# repository identity must be derived from that validated *target* repository,
|
||||
# not from the install-checkout git remote. ``_workspace_repository_slug``
|
||||
# reads ``_local_git_remote_url`` in ``PROJECT_ROOT`` (always Gitea-Tools), so
|
||||
# without this a cross-repo namespace pinned Gitea-Tools while #274 filesystem
|
||||
# membership bound the external root, and ``_enforce_canonical_repository_root``
|
||||
# then failed closed on a self-inflicted identity mismatch. The derived
|
||||
# identity is still authorized by the profile allowlist below (never
|
||||
# self-authorizing). Env-over-profile precedence and fail-closed validation
|
||||
# (existence, git toplevel, resolvable remote identity) are handled by
|
||||
# ``crr``; unconfigured single-repo namespaces keep the install-derived slug.
|
||||
configured_value, configured_source = crr.configured_canonical_root(
|
||||
profile, os.environ
|
||||
)
|
||||
if configured_value:
|
||||
assessment = crr.assess_canonical_repository_root(
|
||||
configured_value=configured_value,
|
||||
source=configured_source,
|
||||
expected_slug=None,
|
||||
process_project_root=PROJECT_ROOT,
|
||||
remote=remote,
|
||||
require_binding=True,
|
||||
)
|
||||
slug = assessment.get("resolved_slug")
|
||||
if assessment.get("block") or not slug:
|
||||
return {
|
||||
"org": None,
|
||||
"repository": None,
|
||||
"reasons": assessment.get("reasons")
|
||||
or [
|
||||
"configured canonical repository root has no resolvable "
|
||||
"repository identity; session repository cannot be "
|
||||
"authorized (fail closed)"
|
||||
],
|
||||
}
|
||||
else:
|
||||
slug = _workspace_repository_slug(remote)
|
||||
scope = session_ctx.assess_repository_scope(
|
||||
workspace_slug=slug,
|
||||
allowed=allowed,
|
||||
@@ -1598,6 +1749,18 @@ def _seed_session_context(
|
||||
"""
|
||||
expected = (profile.get("username") or "").strip() or None
|
||||
trusted = _trusted_session_repository(profile, remote)
|
||||
# #706: pin the configured cross-repository canonical root immutably so a
|
||||
# later call cannot forge/swap it. Store the resolved toplevel so drift
|
||||
# checks compare against the same value the mutation gate validates.
|
||||
configured_value, _crr_source = crr.configured_canonical_root(
|
||||
profile, os.environ
|
||||
)
|
||||
canonical_root_pin = None
|
||||
if configured_value:
|
||||
canonical_root_pin = (
|
||||
crr.resolve_repo_toplevel(configured_value)
|
||||
or os.path.realpath(configured_value)
|
||||
)
|
||||
return session_ctx.seed_session_context_if_unbound(
|
||||
profile_name=profile.get("profile_name") or "",
|
||||
remote=remote,
|
||||
@@ -1608,6 +1771,7 @@ def _seed_session_context(
|
||||
role_kind=_profile_role_kind(profile),
|
||||
expected_username=expected,
|
||||
source=source,
|
||||
canonical_repository_root=canonical_root_pin,
|
||||
)
|
||||
import issue_work_duplicate_gate # noqa: E402
|
||||
import issue_workflow_labels # noqa: E402
|
||||
@@ -2696,8 +2860,13 @@ def gitea_create_issue(
|
||||
return blocked
|
||||
try:
|
||||
with _mutation_stage("preflight_purity"):
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(
|
||||
remote, worktree_path=worktree_path, task="create_issue"
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="create_issue",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
except Exception as exc:
|
||||
typed = _production_guard_block_from_exc(exc, number=None)
|
||||
@@ -2919,7 +3088,14 @@ def gitea_lock_issue(
|
||||
)
|
||||
else:
|
||||
git_state = issue_lock_worktree.read_worktree_git_state(resolved_worktree)
|
||||
verify_preflight_purity(remote, worktree_path=resolved_worktree, task="lock_issue")
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=resolved_worktree,
|
||||
task="lock_issue",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
lock_assessment = issue_lock_worktree.assess_issue_lock_worktree(
|
||||
worktree_path=resolved_worktree,
|
||||
current_branch=git_state.get("current_branch"),
|
||||
@@ -3150,7 +3326,14 @@ def gitea_create_pr(
|
||||
if blocked:
|
||||
return blocked
|
||||
with _mutation_stage("preflight_purity"):
|
||||
verify_preflight_purity(remote, worktree_path=worktree_path, task="create_pr")
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="create_pr",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
|
||||
# ── Issue Lock Validation (Issue #194 / #196 / #443) ──
|
||||
@@ -7446,9 +7629,9 @@ def gitea_edit_pr(
|
||||
# Closing uses close_pr; non-closing title/body/base edits use edit_pr so
|
||||
# the shared #604 anti-stomp inventory stays consistent with wiring.
|
||||
if closing:
|
||||
verify_preflight_purity(remote, task="close_pr")
|
||||
verify_preflight_purity(remote, task="close_pr", org=org, repo=repo)
|
||||
else:
|
||||
verify_preflight_purity(remote, task="edit_pr")
|
||||
verify_preflight_purity(remote, task="edit_pr", org=org, repo=repo)
|
||||
|
||||
# PR closure is a first-class capability, distinct from retitling or
|
||||
# rebasing edits (#216). Gate BEFORE auth/API setup so a blocked close
|
||||
@@ -7699,7 +7882,8 @@ def gitea_commit_files(
|
||||
branch="",
|
||||
)
|
||||
|
||||
verify_preflight_purity(remote, task="commit_files")
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(remote, task="commit_files", org=org, repo=repo)
|
||||
processed_files, source_proofs = _prepare_commit_payload_files(files)
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
@@ -8814,6 +8998,8 @@ def gitea_cleanup_merged_pr_branch(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="cleanup_merged_pr_branch",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
@@ -9541,7 +9727,9 @@ def gitea_reconcile_merged_cleanups(
|
||||
report["executed"] = False
|
||||
return {"success": True, "performed": False, **report}
|
||||
|
||||
verify_preflight_purity(remote, task="reconcile_merged_cleanups")
|
||||
verify_preflight_purity(
|
||||
remote, task="reconcile_merged_cleanups", org=org, repo=repo
|
||||
)
|
||||
actions: list[dict] = []
|
||||
for entry in report.get("entries") or []:
|
||||
head_branch = entry.get("head_branch") or ""
|
||||
@@ -10070,7 +10258,9 @@ def gitea_reconcile_already_landed_pr(
|
||||
)
|
||||
return result
|
||||
|
||||
verify_preflight_purity(remote, task="reconcile_already_landed_pr")
|
||||
verify_preflight_purity(
|
||||
remote, task="reconcile_already_landed_pr", org=org, repo=repo
|
||||
)
|
||||
|
||||
if post_comment and comment_body.strip():
|
||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
||||
@@ -10341,7 +10531,9 @@ def gitea_reconcile_superseded_by_merged_pr(
|
||||
result["safe_next_action"] = "rerun with explicit mutation flags"
|
||||
return result
|
||||
|
||||
verify_preflight_purity(remote, task="reconcile_close_superseded_pr")
|
||||
verify_preflight_purity(
|
||||
remote, task="reconcile_close_superseded_pr", org=org, repo=repo
|
||||
)
|
||||
|
||||
if post_comment:
|
||||
comment_url = f"{base}/issues/{target_pr_number}/comments"
|
||||
@@ -10444,7 +10636,7 @@ def gitea_close_issue(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, task="close_issue")
|
||||
verify_preflight_purity(remote, task="close_issue", org=org, repo=repo)
|
||||
|
||||
# #529: block closure that buries an unproven non-zero suite exit as an
|
||||
# "expected pre-existing failure". Skipped when no closure report is given.
|
||||
@@ -11393,6 +11585,11 @@ def gitea_acquire_merger_pr_lease(
|
||||
}, lease_provenance=merger_lease_adoption.build_lease_provenance(
|
||||
source=merger_lease_adoption.SOURCE_ACQUIRE_MERGER,
|
||||
comment_id=posted.get("id"),
|
||||
# #742: bind the lease to this native runtime so a later owner-session
|
||||
# finalization can prove it is the same daemon that acquired it.
|
||||
native_token_fingerprint=(
|
||||
mcp_daemon_guard.native_runtime_status().get("token_fingerprint")
|
||||
),
|
||||
))
|
||||
return {
|
||||
"success": True,
|
||||
@@ -11582,6 +11779,191 @@ def gitea_adopt_merger_pr_lease(
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_release_merger_pr_lease(
|
||||
pr_number: int,
|
||||
worktree: str,
|
||||
candidate_head: str,
|
||||
outcome: str = merger_lease_adoption.OUTCOME_RELEASED,
|
||||
reason: str | None = None,
|
||||
issue_number: int | None = None,
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
) -> dict:
|
||||
"""Terminally release or abandon this merger session's own PR lease (#742).
|
||||
|
||||
Sanctioned owner-session finalization for a comment-backed merger lease when
|
||||
the merge does **not** occur (non-retryable merge failure, aborted merge, or
|
||||
a blocker that ends the attempt). Without it a failed merger lease can only
|
||||
expire, blocking the queue for the remainder of its TTL.
|
||||
|
||||
Merger-only and owner-only. The caller must hold the exact in-session lease
|
||||
it is finalizing; foreign-session release, reviewer profiles, and mismatched
|
||||
repository / PR / candidate head / identity / profile / native runtime token
|
||||
fingerprint all fail closed. Reviewer sessions use
|
||||
``gitea_release_reviewer_pr_lease`` instead — that tool is never a merger
|
||||
path.
|
||||
|
||||
Finalization is append-only: it posts a terminal lease marker and never
|
||||
edits or deletes prior ledger comments. It is idempotent — an already
|
||||
terminal lease returns ``already_terminal`` and posts nothing. The PR, its
|
||||
approval, the review decision lock, the branch, and the worktree are all
|
||||
preserved; only the lease is ended.
|
||||
|
||||
Args:
|
||||
pr_number: PR whose merger lease to finalize.
|
||||
outcome: 'released' (clean end) or 'abandoned' (ended under a blocker).
|
||||
reason: Short finalization reason recorded in the marker.
|
||||
candidate_head: Pinned head SHA the lease was acquired against.
|
||||
|
||||
Returns:
|
||||
dict reporting finalization status, reasons, and the marker comment id.
|
||||
"""
|
||||
read_block = _profile_operation_gate("gitea.read")
|
||||
if read_block:
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"reasons": read_block,
|
||||
"permission_report": _permission_block_report("gitea.read"),
|
||||
}
|
||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
||||
if comment_block:
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"reasons": comment_block,
|
||||
"permission_report": _permission_block_report("gitea.pr.comment"),
|
||||
}
|
||||
# Merger-role proof: reviewer profiles hold gitea.pr.comment but never
|
||||
# gitea.pr.merge, so this gate keeps the tool merger-only (#742).
|
||||
merge_block = _profile_operation_gate("gitea.pr.merge")
|
||||
if merge_block:
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"reasons": merge_block,
|
||||
"permission_report": _permission_block_report("gitea.pr.merge"),
|
||||
}
|
||||
|
||||
try:
|
||||
_verify_role_mutation_workspace(
|
||||
remote,
|
||||
worktree=worktree,
|
||||
task="release_merger_pr_lease",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
except RuntimeError as e:
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"reasons": [str(e)],
|
||||
}
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
profile = get_profile()
|
||||
profile_name = profile.get("profile_name") or "unknown"
|
||||
identity = _authenticated_username(h) or profile.get("username") or ""
|
||||
repo_label = f"{o}/{r}"
|
||||
|
||||
session = reviewer_pr_lease.get_session_lease()
|
||||
comments = _fetch_pr_comments(
|
||||
pr_number, remote=remote, host=host, org=org, repo=repo)
|
||||
|
||||
pr_live = api_request(
|
||||
"GET", f"{repo_api_url(h, o, r)}/pulls/{pr_number}", auth) or {}
|
||||
live_head = str(
|
||||
(pr_live.get("head") or {}).get("sha") or pr_live.get("head_sha") or ""
|
||||
).strip()
|
||||
|
||||
assessment = merger_lease_adoption.assess_merger_lease_finalization(
|
||||
comments,
|
||||
pr_number=pr_number,
|
||||
session=session,
|
||||
actor_identity=identity,
|
||||
actor_profile=profile_name,
|
||||
actor_session_id=(session or {}).get("session_id"),
|
||||
repo=repo_label,
|
||||
worktree=worktree,
|
||||
candidate_head=candidate_head,
|
||||
live_head_sha=live_head or None,
|
||||
outcome=outcome,
|
||||
reason=reason,
|
||||
runtime_token_fingerprint=(
|
||||
mcp_daemon_guard.native_runtime_status().get("token_fingerprint")
|
||||
),
|
||||
issue_number=issue_number,
|
||||
)
|
||||
|
||||
if assessment.get("already_terminal"):
|
||||
# Idempotent: the lease is already terminal on the ledger. Clear the
|
||||
# in-session mirror so no stale proof survives, and post nothing.
|
||||
reviewer_pr_lease.clear_session_lease()
|
||||
return {
|
||||
"success": True,
|
||||
"finalized": False,
|
||||
"already_terminal": True,
|
||||
"pr_number": pr_number,
|
||||
"outcome": assessment.get("outcome"),
|
||||
"reasons": [],
|
||||
}
|
||||
|
||||
if not assessment.get("finalize_allowed"):
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"already_terminal": False,
|
||||
"pr_number": pr_number,
|
||||
"reasons": assessment.get("reasons") or [],
|
||||
}
|
||||
|
||||
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
||||
with _audited(
|
||||
"comment_pr",
|
||||
host=h,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
pr_number=pr_number,
|
||||
request_metadata={"source": "release_merger_pr_lease"},
|
||||
):
|
||||
posted = api_request(
|
||||
"POST", comment_url, auth, {"body": assessment["finalization_body"]}
|
||||
)
|
||||
|
||||
if not isinstance(posted, dict) or not posted.get("id"):
|
||||
# The lease is still live on the ledger: keep the in-session mirror so
|
||||
# the owner can retry finalization rather than losing its own proof.
|
||||
return {
|
||||
"success": False,
|
||||
"finalized": False,
|
||||
"reasons": [
|
||||
"terminal lease marker POST failed or returned no comment id "
|
||||
"(lease left intact; retry finalization)"
|
||||
],
|
||||
}
|
||||
|
||||
reviewer_pr_lease.clear_session_lease()
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"finalized": True,
|
||||
"already_terminal": False,
|
||||
"pr_number": pr_number,
|
||||
"outcome": assessment.get("outcome"),
|
||||
"finalization_reason": assessment.get("finalization_reason"),
|
||||
"finalization_comment_id": posted.get("id"),
|
||||
"finalized_lease_comment_id": assessment.get("lease_comment_id"),
|
||||
"candidate_head": assessment.get("candidate_head"),
|
||||
"repo": repo_label,
|
||||
"reasons": [],
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_heartbeat_reviewer_pr_lease(
|
||||
pr_number: int,
|
||||
@@ -12411,10 +12793,13 @@ def gitea_create_issue_comment(
|
||||
# allowed while an author holds a different implementation lock.
|
||||
# Scope ownership for source edits is enforced via branch/worktree
|
||||
# binding + root diagnostic checks (#683).
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="comment_issue",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
except Exception as exc:
|
||||
typed = _production_guard_block_from_exc(
|
||||
@@ -14550,7 +14935,14 @@ def gitea_mark_issue(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, worktree_path=worktree_path, task="mark_issue")
|
||||
# #735: forward explicit org/repo into shared anti-stomp preflight.
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="mark_issue",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
@@ -14640,7 +15032,7 @@ def gitea_post_heartbeat(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, task="post_heartbeat")
|
||||
verify_preflight_purity(remote, task="post_heartbeat", org=org, repo=repo)
|
||||
active_profile = profile or get_profile().get("profile_name")
|
||||
body = issue_claim_heartbeat.format_heartbeat_body(
|
||||
kind="progress",
|
||||
@@ -15719,7 +16111,9 @@ def gitea_cleanup_stale_claims(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, task="cleanup_stale_claims")
|
||||
verify_preflight_purity(
|
||||
remote, task="cleanup_stale_claims", org=org, repo=repo
|
||||
)
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
@@ -15839,7 +16233,13 @@ def gitea_create_label(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, worktree_path=worktree_path, task="create_label")
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="create_label",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
@@ -15893,7 +16293,13 @@ def gitea_set_issue_labels(
|
||||
)
|
||||
if blocked:
|
||||
return blocked
|
||||
verify_preflight_purity(remote, worktree_path=worktree_path, task="set_issue_labels")
|
||||
verify_preflight_purity(
|
||||
remote,
|
||||
worktree_path=worktree_path,
|
||||
task="set_issue_labels",
|
||||
org=org,
|
||||
repo=repo,
|
||||
)
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
base = repo_api_url(h, o, r)
|
||||
@@ -16266,6 +16672,8 @@ def gitea_resolve_task_capability(
|
||||
"gitea_acquire_merger_pr_lease",
|
||||
"adopt_merger_pr_lease",
|
||||
"gitea_adopt_merger_pr_lease",
|
||||
"release_merger_pr_lease",
|
||||
"gitea_release_merger_pr_lease",
|
||||
"create_branch",
|
||||
"push_branch",
|
||||
"create_pr",
|
||||
|
||||
@@ -20,6 +20,7 @@ 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"
|
||||
SOURCE_RELEASE_MERGER = "gitea_release_merger_pr_lease"
|
||||
|
||||
SANCTIONED_PROVENANCE_SOURCES = frozenset({
|
||||
SOURCE_ADOPT,
|
||||
@@ -30,6 +31,21 @@ SANCTIONED_PROVENANCE_SOURCES = frozenset({
|
||||
|
||||
_MERGER_ADOPTABLE_FRESHNESS = frozenset({"active", "stale_warning"})
|
||||
|
||||
# #742: owner-session terminal finalization of a merger-held lease. Append-only
|
||||
# marker; ledger history is never edited or deleted.
|
||||
MERGER_FINALIZATION_MARKER = "<!-- mcp-merger-lease-final:v1 -->"
|
||||
OUTCOME_RELEASED = "released"
|
||||
OUTCOME_ABANDONED = "abandoned"
|
||||
MERGER_FINALIZATION_OUTCOMES = frozenset({OUTCOME_RELEASED, OUTCOME_ABANDONED})
|
||||
DEFAULT_MERGER_FINALIZATION_REASON = "merge-not-performed"
|
||||
|
||||
# Provenance sources whose in-session lease is merger-owned and therefore
|
||||
# finalizable by its owning merger session.
|
||||
MERGER_OWNED_PROVENANCE_SOURCES = frozenset({
|
||||
SOURCE_ACQUIRE_MERGER,
|
||||
SOURCE_ADOPT,
|
||||
})
|
||||
|
||||
|
||||
def format_adoption_body(
|
||||
*,
|
||||
@@ -97,6 +113,7 @@ def build_lease_provenance(
|
||||
adopted_from_profile: str | None = None,
|
||||
adopted_from_reviewer_identity: str | None = None,
|
||||
adoption_reason: str | None = None,
|
||||
native_token_fingerprint: str | None = None,
|
||||
recorded_at: datetime | None = None,
|
||||
) -> dict[str, Any]:
|
||||
recorded_at = recorded_at or datetime.now(timezone.utc)
|
||||
@@ -117,9 +134,76 @@ def build_lease_provenance(
|
||||
proof["adopted_from_reviewer_identity"] = adopted_from_reviewer_identity
|
||||
if adoption_reason:
|
||||
proof["adoption_reason"] = adoption_reason
|
||||
if native_token_fingerprint:
|
||||
proof["native_token_fingerprint"] = native_token_fingerprint
|
||||
return proof
|
||||
|
||||
|
||||
def assess_acquired_merger_lease_integrity(
|
||||
session: dict[str, Any] | None,
|
||||
) -> list[str]:
|
||||
"""Ownership/integrity reasons blocking a ``SOURCE_ACQUIRE_MERGER`` lease (#742).
|
||||
|
||||
A merger lease minted by ``gitea_acquire_merger_pr_lease`` authorizes an
|
||||
irreversible merge, so it is sanctioned only when the in-session record is
|
||||
complete and self-consistent: comment marker, exact session identity, merger
|
||||
profile/role, repository, PR number, and pinned candidate head. Any missing
|
||||
or contradictory field fails closed — an incomplete record is not proof.
|
||||
"""
|
||||
if not session:
|
||||
return ["no in-session lease recorded"]
|
||||
provenance = session.get("lease_provenance") or {}
|
||||
if not isinstance(provenance, dict):
|
||||
provenance = {}
|
||||
reasons: list[str] = []
|
||||
|
||||
provenance_comment_id = provenance.get("comment_id")
|
||||
session_comment_id = session.get("comment_id")
|
||||
if not (provenance_comment_id or session_comment_id):
|
||||
reasons.append(
|
||||
"acquired merger lease has no comment marker id (comment-backed "
|
||||
"proof required)"
|
||||
)
|
||||
elif (
|
||||
provenance_comment_id is not None
|
||||
and session_comment_id is not None
|
||||
and provenance_comment_id != session_comment_id
|
||||
):
|
||||
reasons.append(
|
||||
"acquired merger lease provenance comment_id does not match the "
|
||||
"session lease comment_id"
|
||||
)
|
||||
|
||||
if not (session.get("session_id") or "").strip():
|
||||
reasons.append("acquired merger lease has no session_id")
|
||||
if not (session.get("reviewer_identity") or "").strip():
|
||||
reasons.append("acquired merger lease has no holder identity")
|
||||
|
||||
profile = (session.get("profile") or "").strip()
|
||||
if not profile:
|
||||
reasons.append("acquired merger lease has no profile")
|
||||
elif "merger" not in profile.lower():
|
||||
reasons.append(
|
||||
f"acquired merger lease profile '{profile}' is not a merger profile "
|
||||
"(merger-only; fail closed)"
|
||||
)
|
||||
|
||||
if not (session.get("repo") or "").strip():
|
||||
reasons.append("acquired merger lease has no repository")
|
||||
|
||||
pr_number = session.get("pr_number")
|
||||
if not isinstance(pr_number, int) or isinstance(pr_number, bool) or pr_number <= 0:
|
||||
reasons.append("acquired merger lease has no valid PR number")
|
||||
|
||||
if not leases._normalize_sha(session.get("candidate_head")):
|
||||
reasons.append(
|
||||
"acquired merger lease has no pinned candidate_head (exact-head "
|
||||
"scoping required)"
|
||||
)
|
||||
|
||||
return reasons
|
||||
|
||||
|
||||
def is_sanctioned_session_lease(session: dict[str, Any] | None) -> bool:
|
||||
if not session:
|
||||
return False
|
||||
@@ -131,6 +215,8 @@ def is_sanctioned_session_lease(session: dict[str, Any] | None) -> bool:
|
||||
return bool(provenance.get("comment_id")) and bool(
|
||||
provenance.get("adopted_from_session_id")
|
||||
)
|
||||
if source == SOURCE_ACQUIRE_MERGER:
|
||||
return not assess_acquired_merger_lease_integrity(session)
|
||||
if source in {SOURCE_ACQUIRE, SOURCE_HEARTBEAT}:
|
||||
return bool(session.get("comment_id") or provenance.get("comment_id"))
|
||||
return False
|
||||
@@ -168,6 +254,8 @@ def describe_session_lease_proof(
|
||||
if source == SOURCE_ADOPT and sanctioned:
|
||||
kind = "sanctioned_adoption"
|
||||
reason = reason or DEFAULT_ADOPTION_REASON
|
||||
elif source == SOURCE_ACQUIRE_MERGER and sanctioned:
|
||||
kind = "sanctioned_acquire_merger"
|
||||
elif source == SOURCE_ACQUIRE and sanctioned:
|
||||
kind = "sanctioned_acquire"
|
||||
elif source == SOURCE_HEARTBEAT and sanctioned:
|
||||
@@ -216,6 +304,7 @@ _SANCTIONED_LEASE_EVIDENCE_RE = re.compile(
|
||||
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_merger|"
|
||||
r"lease_proof_kind\s*[:=]\s*sanctioned_acquire|"
|
||||
r"adoption_comment_id\s*[:=]|"
|
||||
r"sanctioned_adoption|"
|
||||
@@ -247,6 +336,267 @@ def assess_manual_lease_proof_handoff(report_text: str) -> dict[str, Any]:
|
||||
}
|
||||
|
||||
|
||||
def format_merger_finalization_body(
|
||||
*,
|
||||
repo: str,
|
||||
pr_number: int,
|
||||
issue_number: int | None,
|
||||
merger_identity: str,
|
||||
merger_profile: str,
|
||||
merger_session_id: str,
|
||||
worktree: str,
|
||||
candidate_head: str | None,
|
||||
target_branch: str,
|
||||
target_branch_sha: str | None,
|
||||
outcome: str,
|
||||
reason: str,
|
||||
lease_comment_id: int | None,
|
||||
finalized_at: datetime | None = None,
|
||||
) -> str:
|
||||
"""Render the append-only terminal marker for a merger-owned lease (#742).
|
||||
|
||||
The body carries a standard lease marker in a terminal phase, so the
|
||||
existing newest-wins ledger (#577) ends the lease without editing or
|
||||
deleting any prior comment.
|
||||
"""
|
||||
finalized_at = finalized_at or datetime.now(timezone.utc)
|
||||
finalized_text = finalized_at.astimezone(timezone.utc).replace(
|
||||
microsecond=0
|
||||
).isoformat().replace("+00:00", "Z")
|
||||
lease_body = leases.format_lease_body(
|
||||
repo=repo,
|
||||
pr_number=pr_number,
|
||||
issue_number=issue_number,
|
||||
reviewer_identity=merger_identity,
|
||||
profile=merger_profile,
|
||||
session_id=merger_session_id,
|
||||
worktree=worktree,
|
||||
phase=outcome,
|
||||
candidate_head=candidate_head,
|
||||
target_branch=target_branch,
|
||||
target_branch_sha=target_branch_sha,
|
||||
last_activity=finalized_at,
|
||||
blocker=reason,
|
||||
)
|
||||
lines = [
|
||||
MERGER_FINALIZATION_MARKER,
|
||||
f"finalized_at: {finalized_text}",
|
||||
f"finalized_by_identity: {merger_identity}",
|
||||
f"finalized_by_profile: {merger_profile}",
|
||||
f"finalized_by_session_id: {merger_session_id}",
|
||||
f"finalization_outcome: {outcome}",
|
||||
f"finalization_reason: {reason}",
|
||||
f"finalized_lease_comment_id: {lease_comment_id or 'none'}",
|
||||
lease_body,
|
||||
]
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def is_merger_finalization_comment(body: str) -> bool:
|
||||
return MERGER_FINALIZATION_MARKER in (body or "")
|
||||
|
||||
|
||||
def assess_merger_lease_finalization(
|
||||
comments: list[dict],
|
||||
*,
|
||||
pr_number: int,
|
||||
session: dict[str, Any] | None,
|
||||
actor_identity: str,
|
||||
actor_profile: str,
|
||||
actor_session_id: str | None,
|
||||
repo: str,
|
||||
worktree: str,
|
||||
candidate_head: str | None,
|
||||
live_head_sha: str | None = None,
|
||||
outcome: str = OUTCOME_RELEASED,
|
||||
reason: str | None = None,
|
||||
runtime_token_fingerprint: str | None = None,
|
||||
issue_number: int | None = None,
|
||||
target_branch: str = "master",
|
||||
target_branch_sha: str | None = None,
|
||||
now: datetime | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Decide whether a merger may terminally finalize its own lease (#742).
|
||||
|
||||
Owner-session only: the caller must hold the exact comment-backed merger
|
||||
lease it is finalizing. Foreign sessions, reviewer profiles, and mismatched
|
||||
repository/PR/head/token-fingerprint callers fail closed. Already-terminal
|
||||
leases return ``already_terminal`` so repeat calls are idempotent and post
|
||||
no second marker.
|
||||
"""
|
||||
now = now or datetime.now(timezone.utc)
|
||||
reasons: list[str] = []
|
||||
outcome = (outcome or "").strip().lower()
|
||||
finalization_reason = (reason or "").strip() or DEFAULT_MERGER_FINALIZATION_REASON
|
||||
|
||||
if outcome not in MERGER_FINALIZATION_OUTCOMES:
|
||||
reasons.append(
|
||||
f"outcome '{outcome or 'none'}' is not a terminal merger "
|
||||
f"finalization outcome ({sorted(MERGER_FINALIZATION_OUTCOMES)})"
|
||||
)
|
||||
|
||||
if "merger" not in (actor_profile or "").lower():
|
||||
reasons.append(
|
||||
f"profile '{actor_profile or 'unknown'}' is not a merger profile; "
|
||||
"merger lease finalization is merger-only (fail closed). Reviewer "
|
||||
"sessions use gitea_release_reviewer_pr_lease."
|
||||
)
|
||||
|
||||
session = session or None
|
||||
provenance = (session or {}).get("lease_provenance") or {}
|
||||
if not isinstance(provenance, dict):
|
||||
provenance = {}
|
||||
source = (provenance.get("source") or "").strip()
|
||||
|
||||
if not session:
|
||||
reasons.append(
|
||||
"no in-session merger lease recorded; only the owning session may "
|
||||
"finalize a merger lease"
|
||||
)
|
||||
elif source not in MERGER_OWNED_PROVENANCE_SOURCES:
|
||||
reasons.append(
|
||||
f"in-session lease provenance '{source or 'none'}' is not a "
|
||||
"merger-owned lease; refusing to finalize"
|
||||
)
|
||||
elif not is_sanctioned_session_lease(session):
|
||||
reasons.extend(
|
||||
assess_acquired_merger_lease_integrity(session)
|
||||
if source == SOURCE_ACQUIRE_MERGER
|
||||
else ["in-session merger lease lacks sanctioned provenance"]
|
||||
)
|
||||
|
||||
pinned = leases._normalize_sha(candidate_head)
|
||||
live = leases._normalize_sha(live_head_sha)
|
||||
if not pinned:
|
||||
reasons.append(
|
||||
"candidate_head is required for merger lease finalization "
|
||||
"(exact-head scoping; fail closed)"
|
||||
)
|
||||
if live and pinned and live != pinned:
|
||||
reasons.append(
|
||||
"candidate_head does not match live PR head (fail closed)"
|
||||
)
|
||||
|
||||
if session:
|
||||
session_sid = (session.get("session_id") or "").strip()
|
||||
actor_sid = (actor_session_id or "").strip()
|
||||
if not actor_sid or session_sid != actor_sid:
|
||||
reasons.append(
|
||||
"session_id does not match the in-session merger lease owner; "
|
||||
"foreign-session release is not permitted (fail closed)"
|
||||
)
|
||||
if session.get("pr_number") != pr_number:
|
||||
reasons.append(
|
||||
f"in-session merger lease is for PR #{session.get('pr_number')}, "
|
||||
f"not #{pr_number}"
|
||||
)
|
||||
session_repo = (session.get("repo") or "").strip()
|
||||
if session_repo and session_repo != (repo or "").strip():
|
||||
reasons.append(
|
||||
f"in-session merger lease repository '{session_repo}' does not "
|
||||
f"match '{repo}' (fail closed)"
|
||||
)
|
||||
session_head = leases._normalize_sha(session.get("candidate_head"))
|
||||
if pinned and session_head and session_head != pinned:
|
||||
reasons.append(
|
||||
"in-session merger lease candidate_head does not match the "
|
||||
"supplied candidate_head (fail closed)"
|
||||
)
|
||||
session_identity = (session.get("reviewer_identity") or "").strip()
|
||||
if session_identity and session_identity != (actor_identity or "").strip():
|
||||
reasons.append(
|
||||
"authenticated identity does not match the in-session merger "
|
||||
"lease holder (fail closed)"
|
||||
)
|
||||
session_profile = (session.get("profile") or "").strip()
|
||||
if session_profile and session_profile != (actor_profile or "").strip():
|
||||
reasons.append(
|
||||
"active profile does not match the in-session merger lease "
|
||||
"profile (fail closed)"
|
||||
)
|
||||
recorded_fingerprint = (
|
||||
provenance.get("native_token_fingerprint") or ""
|
||||
).strip()
|
||||
live_fingerprint = (runtime_token_fingerprint or "").strip()
|
||||
if (
|
||||
recorded_fingerprint
|
||||
and live_fingerprint
|
||||
and recorded_fingerprint != live_fingerprint
|
||||
):
|
||||
reasons.append(
|
||||
"native runtime token fingerprint does not match the one "
|
||||
"recorded when the merger lease was acquired (fail closed)"
|
||||
)
|
||||
|
||||
lease_comment_id = (session or {}).get("comment_id") or provenance.get("comment_id")
|
||||
entries = leases._lease_entries(comments, pr_number=pr_number)
|
||||
if session and lease_comment_id is not None:
|
||||
if not any(entry.get("comment_id") == lease_comment_id for entry in entries):
|
||||
reasons.append(
|
||||
f"lease marker comment {lease_comment_id} is not present on PR "
|
||||
f"#{pr_number} (fail closed)"
|
||||
)
|
||||
|
||||
active = leases.find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
|
||||
already_terminal = False
|
||||
if active:
|
||||
owner_session = (active.get("session_id") or "").strip()
|
||||
if owner_session and owner_session != (actor_session_id or "").strip():
|
||||
reasons.append(
|
||||
f"active PR lease is owned by session_id={owner_session}; a "
|
||||
"merger may only finalize its own lease (fail closed)"
|
||||
)
|
||||
else:
|
||||
newest = entries[-1] if entries else None
|
||||
newest_phase = ((newest or {}).get("phase") or "").strip().lower()
|
||||
if newest and newest_phase in leases._TERMINAL_PHASES:
|
||||
already_terminal = True
|
||||
elif not entries:
|
||||
reasons.append(
|
||||
f"no comment-backed lease marker found on PR #{pr_number}"
|
||||
)
|
||||
|
||||
finalize_allowed = not reasons and not already_terminal
|
||||
body = None
|
||||
if finalize_allowed and session:
|
||||
body = format_merger_finalization_body(
|
||||
repo=(session.get("repo") or repo),
|
||||
pr_number=pr_number,
|
||||
issue_number=(
|
||||
issue_number
|
||||
if issue_number is not None
|
||||
else session.get("issue_number")
|
||||
),
|
||||
merger_identity=actor_identity,
|
||||
merger_profile=actor_profile,
|
||||
merger_session_id=(actor_session_id or ""),
|
||||
worktree=worktree or (session.get("worktree") or ""),
|
||||
candidate_head=pinned,
|
||||
target_branch=(
|
||||
session.get("target_branch") or target_branch or "master"
|
||||
),
|
||||
target_branch_sha=(
|
||||
session.get("target_branch_sha") or target_branch_sha
|
||||
),
|
||||
outcome=outcome,
|
||||
reason=finalization_reason,
|
||||
lease_comment_id=lease_comment_id,
|
||||
finalized_at=now,
|
||||
)
|
||||
|
||||
return {
|
||||
"finalize_allowed": finalize_allowed,
|
||||
"already_terminal": already_terminal,
|
||||
"reasons": reasons,
|
||||
"outcome": outcome,
|
||||
"finalization_reason": finalization_reason,
|
||||
"active_lease": active,
|
||||
"finalization_body": body,
|
||||
"lease_comment_id": lease_comment_id,
|
||||
"candidate_head": pinned,
|
||||
}
|
||||
|
||||
|
||||
def assess_adopt_merger_lease(
|
||||
comments: list[dict],
|
||||
*,
|
||||
|
||||
@@ -109,8 +109,17 @@ def resolve_namespace_mutation_context(
|
||||
session_lease_worktree: str | None = None,
|
||||
worktree: str | None = None,
|
||||
profile_name: str | None = None,
|
||||
configured_canonical_root: str | None = None,
|
||||
) -> dict:
|
||||
"""Shared workspace resolution for runtime_context and mutation guards."""
|
||||
"""Shared workspace resolution for runtime_context and mutation guards.
|
||||
|
||||
When *configured_canonical_root* is supplied (a cross-repository namespace
|
||||
bound to an external target repository, #706), the canonical repository root
|
||||
is that configured target rather than the MCP install checkout. This keeps
|
||||
the branches-only / worktree-membership guards (#274) evaluating against the
|
||||
repository the namespace actually mutates. Without it the single-repo
|
||||
default is preserved: the canonical root follows the process checkout.
|
||||
"""
|
||||
demotions: list[str] = []
|
||||
workspace, binding_source = resolve_namespace_workspace(
|
||||
role_kind=role_kind,
|
||||
@@ -132,7 +141,11 @@ def resolve_namespace_mutation_context(
|
||||
env=env,
|
||||
profile_name=profile_name,
|
||||
)
|
||||
canonical_root = amw.resolve_canonical_repo_root(process_root, process_root)
|
||||
configured = (configured_canonical_root or "").strip()
|
||||
if configured:
|
||||
canonical_root = os.path.realpath(configured)
|
||||
else:
|
||||
canonical_root = amw.resolve_canonical_repo_root(process_root, process_root)
|
||||
return {
|
||||
"workspace_path": workspace,
|
||||
"workspace_binding_source": binding_source,
|
||||
@@ -258,6 +271,7 @@ def assess_namespace_mutation_workspace(
|
||||
session_lease_worktree: str | None = None,
|
||||
profile_name: str | None = None,
|
||||
current_branch: str | None = None,
|
||||
configured_canonical_root: str | None = None,
|
||||
) -> dict:
|
||||
"""Evaluate namespace workspace binding before preflight/mutation."""
|
||||
ctx = resolve_namespace_mutation_context(
|
||||
@@ -268,6 +282,7 @@ def assess_namespace_mutation_workspace(
|
||||
env=env,
|
||||
session_lease_worktree=session_lease_worktree,
|
||||
profile_name=profile_name,
|
||||
configured_canonical_root=configured_canonical_root,
|
||||
)
|
||||
mutation_workspace = ctx["workspace_path"]
|
||||
binding_source = ctx["workspace_binding_source"]
|
||||
|
||||
+54
-3
@@ -20,7 +20,11 @@ _FIELD_RE = re.compile(
|
||||
re.IGNORECASE | re.MULTILINE,
|
||||
)
|
||||
|
||||
_TERMINAL_REVIEWER_PHASES = frozenset({"done", "released", "blocked"})
|
||||
# Must mirror reviewer_pr_lease._TERMINAL_PHASES: both modules read the same
|
||||
# append-only lease markers, so a phase that is terminal in one and active in
|
||||
# the other yields two conflicting truths for the same comment (#742 review
|
||||
# 460). "abandoned" is the owner-session merger finalization outcome.
|
||||
_TERMINAL_REVIEWER_PHASES = frozenset({"done", "released", "blocked", "abandoned"})
|
||||
_ACTIVE_REVIEWER_PHASES = frozenset({
|
||||
"claimed",
|
||||
"validating",
|
||||
@@ -153,25 +157,72 @@ def _lease_phase_active(lease: dict, *, active_phases: frozenset[str]) -> bool:
|
||||
))
|
||||
|
||||
|
||||
def _reviewer_chain_key(lease: dict) -> tuple | None:
|
||||
"""Identity of the lease chain a reviewer marker belongs to (#742).
|
||||
|
||||
A chain is one session's claim → heartbeat → terminal sequence, keyed by
|
||||
repository, PR, candidate head, session id, identity, and profile. Returns
|
||||
None when any component is missing: an incomplete or malformed marker has
|
||||
no provable chain, so it can neither be cancelled by nor cancel anything.
|
||||
"""
|
||||
raw = lease.get("raw_fields") or {}
|
||||
repo = (raw.get("repo") or "").strip().lower()
|
||||
session_id = (lease.get("session_id") or "").strip()
|
||||
identity = (lease.get("reviewer_identity") or "").strip()
|
||||
profile = (lease.get("profile") or "").strip()
|
||||
head = lease.get("candidate_head")
|
||||
pr_number = lease.get("pr_number")
|
||||
if not (repo and session_id and identity and profile and head and pr_number):
|
||||
return None
|
||||
return (repo, pr_number, head, session_id, identity, profile)
|
||||
|
||||
|
||||
def _chain_terminated_after(entries: list[dict], index: int) -> bool:
|
||||
"""True when a later marker terminates the chain of ``entries[index]``.
|
||||
|
||||
Append-only newest-wins (#577 semantics, chain-scoped for #742): a terminal
|
||||
marker ends only its *own* claim, so a foreign, forged, or malformed
|
||||
terminal marker cannot cancel another session's valid active lease.
|
||||
"""
|
||||
key = _reviewer_chain_key(entries[index])
|
||||
if key is None:
|
||||
return False
|
||||
for later in entries[index + 1:]:
|
||||
if (later.get("phase") or "").strip().lower() not in _TERMINAL_REVIEWER_PHASES:
|
||||
continue
|
||||
if _reviewer_chain_key(later) == key:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def find_active_reviewer_lease(
|
||||
comments: list[dict],
|
||||
*,
|
||||
pr_number: int,
|
||||
now: datetime | None = None,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Return the newest unexpired reviewer lease for *pr_number*, if any."""
|
||||
"""Return the newest unexpired, non-terminated reviewer lease for *pr_number*.
|
||||
|
||||
Walking backward past a terminal marker used to resurrect the older claim of
|
||||
the very chain that marker ended, so a released/abandoned finalization still
|
||||
read as an active lease here while ``reviewer_pr_lease`` reported it ended
|
||||
(#742). A claim is now skipped when a later marker terminates its own chain.
|
||||
"""
|
||||
now = now or datetime.now(timezone.utc)
|
||||
candidates = [
|
||||
entry for entry in _comment_entries(comments, pr_number=pr_number)
|
||||
if entry.get("lease_kind") == "reviewer"
|
||||
]
|
||||
for lease in reversed(candidates):
|
||||
for index in range(len(candidates) - 1, -1, -1):
|
||||
lease = candidates[index]
|
||||
if _lease_expired(lease, now=now):
|
||||
continue
|
||||
phase = (lease.get("phase") or "").strip().lower()
|
||||
if phase in _TERMINAL_REVIEWER_PHASES:
|
||||
continue
|
||||
if phase in _ACTIVE_REVIEWER_PHASES or phase:
|
||||
if _chain_terminated_after(candidates, index):
|
||||
continue
|
||||
return lease
|
||||
return None
|
||||
|
||||
|
||||
@@ -16,7 +16,10 @@ _FIELD_RE = re.compile(
|
||||
)
|
||||
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
|
||||
|
||||
_TERMINAL_PHASES = frozenset({"done", "released", "blocked"})
|
||||
# "abandoned" is the owner-session merger finalization outcome (#742). Without
|
||||
# it here, an abandoned marker would fall through to the generic non-empty
|
||||
# phase branch and keep re-arming the lease as active.
|
||||
_TERMINAL_PHASES = frozenset({"done", "released", "blocked", "abandoned"})
|
||||
_ACTIVE_PHASES = frozenset({
|
||||
"claimed",
|
||||
"validating",
|
||||
|
||||
@@ -32,6 +32,7 @@ class _SessionContext:
|
||||
expected_username: str | None
|
||||
source: str
|
||||
pid: int
|
||||
canonical_repository_root: str | None = None
|
||||
|
||||
def as_dict(self) -> dict[str, Any]:
|
||||
return {
|
||||
@@ -45,6 +46,7 @@ class _SessionContext:
|
||||
"expected_username": self.expected_username,
|
||||
"source": self.source,
|
||||
"pid": self.pid,
|
||||
"canonical_repository_root": self.canonical_repository_root,
|
||||
}
|
||||
|
||||
|
||||
@@ -143,6 +145,7 @@ def bind_session_context(
|
||||
role_kind: str | None = None,
|
||||
expected_username: str | None = None,
|
||||
source: str = "bind",
|
||||
canonical_repository_root: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Atomically bind/re-bind context (the explicit activation path)."""
|
||||
with _SESSION_CONTEXT_LOCK:
|
||||
@@ -156,6 +159,7 @@ def bind_session_context(
|
||||
role_kind=role_kind,
|
||||
expected_username=expected_username,
|
||||
source=source,
|
||||
canonical_repository_root=canonical_repository_root,
|
||||
)
|
||||
|
||||
|
||||
@@ -170,6 +174,7 @@ def _bind_session_context_unlocked(
|
||||
role_kind: str | None,
|
||||
expected_username: str | None,
|
||||
source: str,
|
||||
canonical_repository_root: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Store a complete immutable context while the caller holds the lock."""
|
||||
global _SESSION_CONTEXT
|
||||
@@ -184,6 +189,7 @@ def _bind_session_context_unlocked(
|
||||
expected_username=(expected_username or "").strip() or None,
|
||||
source=source,
|
||||
pid=os.getpid(),
|
||||
canonical_repository_root=(canonical_repository_root or "").strip() or None,
|
||||
)
|
||||
return _SESSION_CONTEXT.as_dict()
|
||||
|
||||
@@ -199,6 +205,7 @@ def seed_session_context_if_unbound(
|
||||
role_kind: str | None = None,
|
||||
expected_username: str | None = None,
|
||||
source: str = "seed",
|
||||
canonical_repository_root: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Atomically bind only when this process has no current context.
|
||||
|
||||
@@ -219,6 +226,7 @@ def seed_session_context_if_unbound(
|
||||
role_kind=role_kind,
|
||||
expected_username=expected_username,
|
||||
source=source,
|
||||
canonical_repository_root=canonical_repository_root,
|
||||
)
|
||||
return _SESSION_CONTEXT.as_dict()
|
||||
|
||||
@@ -232,6 +240,7 @@ def assess_session_context(
|
||||
repository: str | None = None,
|
||||
org: str | None = None,
|
||||
expected_username: str | None = None,
|
||||
canonical_repository_root: str | None = None,
|
||||
require_bound: bool = False,
|
||||
require_complete: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
@@ -272,6 +281,7 @@ def assess_session_context(
|
||||
live_identity = (identity or "").strip() or None
|
||||
live_repo = (repository or "").strip() or None
|
||||
live_org = (org or "").strip() or None
|
||||
live_canonical = (canonical_repository_root or "").strip() or None
|
||||
|
||||
if ctx.get("profile_name") and live_profile and live_profile != ctx.get("profile_name"):
|
||||
reasons.append(
|
||||
@@ -307,6 +317,13 @@ def assess_session_context(
|
||||
f"org drift: live '{live_org}' != bound "
|
||||
f"'{ctx.get('org')}' (fail closed)"
|
||||
)
|
||||
bound_canonical = ctx.get("canonical_repository_root")
|
||||
if bound_canonical and live_canonical and live_canonical != bound_canonical:
|
||||
reasons.append(
|
||||
f"canonical repository root drift: live '{live_canonical}' != bound "
|
||||
f"'{bound_canonical}' (forged or conflicting cross-repository "
|
||||
"binding, fail closed)"
|
||||
)
|
||||
|
||||
expected = expected_username or ctx.get("expected_username")
|
||||
if expected and live_identity and live_identity != expected:
|
||||
@@ -580,6 +597,7 @@ def mutation_context_audit_fields(
|
||||
"session_org": data.get("org"),
|
||||
"session_role_kind": data.get("role_kind"),
|
||||
"session_context_source": data.get("source"),
|
||||
"session_canonical_repository_root": data.get("canonical_repository_root"),
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -137,6 +137,17 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
|
||||
"permission": "gitea.pr.comment",
|
||||
"role": "merger",
|
||||
},
|
||||
# #742: owner-session terminal release/abandon of a merger-held lease when
|
||||
# the merge does not occur. Apply path posts an append-only terminal lease
|
||||
# marker (gitea.pr.comment); merger-only, never a reviewer path.
|
||||
"release_merger_pr_lease": {
|
||||
"permission": "gitea.pr.comment",
|
||||
"role": "merger",
|
||||
},
|
||||
"gitea_release_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).
|
||||
"cleanup_obsolete_reviewer_comment_lease": {
|
||||
|
||||
@@ -0,0 +1,423 @@
|
||||
"""Issue #706: immutable startup/profile canonical_repository_root capability.
|
||||
|
||||
Cross-repository MCP namespaces (e.g. ``eagenda-author``) must bind their
|
||||
canonical repository root to the *configured target repository*, not to the
|
||||
Gitea-Tools install checkout the server script lives in. These tests prove:
|
||||
|
||||
* the configured binding is resolved from profile/env with env precedence,
|
||||
* the target repository identity and git common-directory membership are
|
||||
validated (AC3),
|
||||
* the branches-only guard (#274) is enforced *inside the target repo* (AC4),
|
||||
* missing / conflicting / forged bindings fail closed (AC5),
|
||||
* prgs and mdcps namespaces stay simultaneously isolated (AC6),
|
||||
* the session binding pins the canonical root immutably,
|
||||
* no weakening of the install-root single-repo default (AC8).
|
||||
|
||||
Uses two distinct real git repositories/worktrees (AC7).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import canonical_repository_root as crr # noqa: E402
|
||||
import gitea_config # noqa: E402
|
||||
import namespace_workspace_binding as nwb # noqa: E402
|
||||
import session_context_binding as session_ctx # noqa: E402
|
||||
|
||||
|
||||
def _git(cwd: str, *args: str) -> str:
|
||||
res = subprocess.run(
|
||||
["git", "-C", cwd, *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
return res.stdout.strip()
|
||||
|
||||
|
||||
def _init_repo(path: Path, remote_url: str, *, remote_name: str = "origin") -> str:
|
||||
"""Create a real git repo with one commit and a remote; return realpath root."""
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
_git(str(path), "init", "-q")
|
||||
_git(str(path), "config", "user.email", "[email protected]")
|
||||
_git(str(path), "config", "user.name", "Test")
|
||||
_git(str(path), "remote", "add", remote_name, remote_url)
|
||||
(path / "README.md").write_text("seed\n")
|
||||
_git(str(path), "add", "README.md")
|
||||
_git(str(path), "commit", "-q", "-m", "seed")
|
||||
return os.path.realpath(str(path))
|
||||
|
||||
|
||||
def _add_worktree(repo_root: str, worktree_path: Path, branch: str) -> str:
|
||||
_git(repo_root, "worktree", "add", "-q", "-b", branch, str(worktree_path))
|
||||
return os.path.realpath(str(worktree_path))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# configured_canonical_root: resolution + precedence
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestConfiguredCanonicalRoot(unittest.TestCase):
|
||||
def test_unconfigured_returns_none(self):
|
||||
value, source = crr.configured_canonical_root({}, {})
|
||||
self.assertIsNone(value)
|
||||
self.assertIsNone(source)
|
||||
|
||||
def test_profile_field_used(self):
|
||||
value, source = crr.configured_canonical_root(
|
||||
{"canonical_repository_root": "/repo/eAgenda"}, {}
|
||||
)
|
||||
self.assertEqual(value, "/repo/eAgenda")
|
||||
self.assertIn("profile", source)
|
||||
|
||||
def test_env_overrides_profile(self):
|
||||
value, source = crr.configured_canonical_root(
|
||||
{"canonical_repository_root": "/repo/eAgenda"},
|
||||
{crr.CANONICAL_ROOT_ENV: "/repo/other"},
|
||||
)
|
||||
self.assertEqual(value, "/repo/other")
|
||||
self.assertIn(crr.CANONICAL_ROOT_ENV, source)
|
||||
|
||||
def test_blank_values_ignored(self):
|
||||
value, source = crr.configured_canonical_root(
|
||||
{"canonical_repository_root": " "},
|
||||
{crr.CANONICAL_ROOT_ENV: ""},
|
||||
)
|
||||
self.assertIsNone(value)
|
||||
self.assertIsNone(source)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# assess_canonical_repository_root: default (single-repo) path
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestUnconfiguredFallback(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.tmp = self._tmp.name
|
||||
|
||||
def tearDown(self):
|
||||
self._tmp.cleanup()
|
||||
|
||||
def test_unconfigured_falls_back_to_process_root(self):
|
||||
root = _init_repo(
|
||||
Path(self.tmp) / "install",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git",
|
||||
)
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=None,
|
||||
source=None,
|
||||
expected_slug=None,
|
||||
process_project_root=root,
|
||||
)
|
||||
self.assertTrue(got["proven"])
|
||||
self.assertFalse(got["block"])
|
||||
self.assertFalse(got["configured"])
|
||||
self.assertEqual(got["canonical_repo_root"], root)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# assess_canonical_repository_root: configured cross-repo path
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestConfiguredCrossRepo(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.tmp = Path(self._tmp.name)
|
||||
self.install = _init_repo(
|
||||
self.tmp / "Gitea-Tools",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git",
|
||||
)
|
||||
self.target = _init_repo(
|
||||
self.tmp / "mcp-control-plane",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/mcp-control-plane.git",
|
||||
)
|
||||
|
||||
def tearDown(self):
|
||||
self._tmp.cleanup()
|
||||
|
||||
def test_valid_configured_root_binds_to_target(self):
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=self.target,
|
||||
source="profile canonical_repository_root",
|
||||
expected_slug="Scaled-Tech-Consulting/mcp-control-plane",
|
||||
process_project_root=self.install,
|
||||
)
|
||||
self.assertTrue(got["proven"], got.get("reasons"))
|
||||
self.assertFalse(got["block"])
|
||||
self.assertTrue(got["configured"])
|
||||
self.assertEqual(got["canonical_repo_root"], self.target)
|
||||
self.assertNotEqual(got["canonical_repo_root"], self.install)
|
||||
|
||||
def test_missing_path_fails_closed(self):
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=str(self.tmp / "does-not-exist"),
|
||||
source="profile canonical_repository_root",
|
||||
expected_slug=None,
|
||||
process_project_root=self.install,
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
self.assertFalse(got["proven"])
|
||||
self.assertTrue(any("exist" in r for r in got["reasons"]))
|
||||
|
||||
def test_non_git_path_fails_closed(self):
|
||||
plain = self.tmp / "plain"
|
||||
plain.mkdir()
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=str(plain),
|
||||
source="profile canonical_repository_root",
|
||||
expected_slug=None,
|
||||
process_project_root=self.install,
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
self.assertTrue(any("git" in r for r in got["reasons"]))
|
||||
|
||||
def test_identity_mismatch_fails_closed(self):
|
||||
# Configured root is the install repo, but session expects the target
|
||||
# repo identity: a forged/conflicting binding.
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=self.install,
|
||||
source="profile canonical_repository_root",
|
||||
expected_slug="Scaled-Tech-Consulting/mcp-control-plane",
|
||||
process_project_root=self.install,
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
self.assertTrue(any("identity" in r for r in got["reasons"]))
|
||||
|
||||
def test_unprovable_identity_blocks_when_required(self):
|
||||
noremote = _init_repo(self.tmp / "noremote-src", "x", remote_name="origin")
|
||||
_git(noremote, "remote", "remove", "origin")
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=noremote,
|
||||
source="profile canonical_repository_root",
|
||||
expected_slug="Scaled-Tech-Consulting/mcp-control-plane",
|
||||
process_project_root=self.install,
|
||||
require_binding=True,
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
|
||||
def test_repository_identity_slug_reads_remote(self):
|
||||
self.assertEqual(
|
||||
crr.repository_identity_slug(self.target),
|
||||
"Scaled-Tech-Consulting/mcp-control-plane",
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Workspace membership + branches-only enforced inside the TARGET repo (AC4)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestNamespaceContextUsesConfiguredRoot(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.tmp = Path(self._tmp.name)
|
||||
self.install = _init_repo(
|
||||
self.tmp / "Gitea-Tools",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git",
|
||||
)
|
||||
self.target = _init_repo(
|
||||
self.tmp / "mcp-control-plane",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/mcp-control-plane.git",
|
||||
)
|
||||
(Path(self.target) / "branches").mkdir()
|
||||
self.target_wt = _add_worktree(
|
||||
self.target,
|
||||
Path(self.target) / "branches" / "author-issue-1",
|
||||
"feat/issue-1",
|
||||
)
|
||||
|
||||
def tearDown(self):
|
||||
self._tmp.cleanup()
|
||||
|
||||
def test_context_canonical_root_is_configured_target(self):
|
||||
ctx = nwb.resolve_namespace_mutation_context(
|
||||
role_kind="author",
|
||||
worktree_path=self.target_wt,
|
||||
process_project_root=self.install,
|
||||
env={},
|
||||
configured_canonical_root=self.target,
|
||||
)
|
||||
self.assertEqual(ctx["canonical_repo_root"], self.target)
|
||||
self.assertFalse(ctx["roots_aligned"])
|
||||
|
||||
def test_target_worktree_is_member_of_target_root(self):
|
||||
got = nwb.amw.assess_workspace_repo_membership(
|
||||
workspace_path=self.target_wt,
|
||||
canonical_repo_root=self.target,
|
||||
)
|
||||
self.assertTrue(got["proven"], got.get("reasons"))
|
||||
|
||||
def test_target_worktree_not_member_of_install_root(self):
|
||||
got = nwb.amw.assess_workspace_repo_membership(
|
||||
workspace_path=self.target_wt,
|
||||
canonical_repo_root=self.install,
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
|
||||
def test_branches_guard_passes_inside_target(self):
|
||||
assessment = nwb.assess_namespace_mutation_workspace(
|
||||
role_kind="author",
|
||||
worktree_path=self.target_wt,
|
||||
worktree=None,
|
||||
process_project_root=self.install,
|
||||
env={},
|
||||
current_branch="feat/issue-1",
|
||||
configured_canonical_root=self.target,
|
||||
)
|
||||
self.assertFalse(assessment["block"], assessment.get("reasons"))
|
||||
self.assertEqual(assessment["canonical_repo_root"], self.target)
|
||||
|
||||
def test_target_control_checkout_blocks_branches_guard(self):
|
||||
assessment = nwb.assess_namespace_mutation_workspace(
|
||||
role_kind="author",
|
||||
worktree_path=self.target, # stable target checkout, not branches/
|
||||
worktree=None,
|
||||
process_project_root=self.install,
|
||||
env={},
|
||||
current_branch="master",
|
||||
configured_canonical_root=self.target,
|
||||
)
|
||||
self.assertTrue(assessment["block"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Immutable session pin (AC1/AC5)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestSessionCanonicalRootPin(unittest.TestCase):
|
||||
def setUp(self):
|
||||
os.environ["PYTEST_CURRENT_TEST"] = "t"
|
||||
session_ctx._reset_session_context_for_testing()
|
||||
|
||||
def tearDown(self):
|
||||
session_ctx._reset_session_context_for_testing()
|
||||
|
||||
def test_seed_stores_canonical_root(self):
|
||||
ctx = session_ctx.seed_session_context_if_unbound(
|
||||
profile_name="mcp-control-plane-author",
|
||||
remote="prgs",
|
||||
host="gitea.prgs.cc",
|
||||
identity="svc",
|
||||
repository="mcp-control-plane",
|
||||
org="Scaled-Tech-Consulting",
|
||||
canonical_repository_root="/repo/mcp-control-plane",
|
||||
)
|
||||
self.assertEqual(ctx["canonical_repository_root"], "/repo/mcp-control-plane")
|
||||
|
||||
def test_canonical_root_drift_fails_closed(self):
|
||||
session_ctx.seed_session_context_if_unbound(
|
||||
profile_name="mcp-control-plane-author",
|
||||
remote="prgs",
|
||||
host="gitea.prgs.cc",
|
||||
identity="svc",
|
||||
repository="mcp-control-plane",
|
||||
org="Scaled-Tech-Consulting",
|
||||
canonical_repository_root="/repo/mcp-control-plane",
|
||||
)
|
||||
got = session_ctx.assess_session_context(
|
||||
profile_name="mcp-control-plane-author",
|
||||
remote="prgs",
|
||||
canonical_repository_root="/repo/forged-elsewhere",
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
self.assertTrue(any("canonical" in r for r in got["reasons"]))
|
||||
|
||||
def test_matching_canonical_root_passes(self):
|
||||
session_ctx.seed_session_context_if_unbound(
|
||||
profile_name="mcp-control-plane-author",
|
||||
remote="prgs",
|
||||
host="gitea.prgs.cc",
|
||||
identity="svc",
|
||||
repository="mcp-control-plane",
|
||||
org="Scaled-Tech-Consulting",
|
||||
canonical_repository_root="/repo/mcp-control-plane",
|
||||
)
|
||||
got = session_ctx.assess_session_context(
|
||||
profile_name="mcp-control-plane-author",
|
||||
remote="prgs",
|
||||
canonical_repository_root="/repo/mcp-control-plane",
|
||||
)
|
||||
self.assertFalse(got["block"], got["reasons"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Simultaneous prgs / mdcps isolation (AC6)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestSimultaneousIsolation(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.tmp = Path(self._tmp.name)
|
||||
self.prgs = _init_repo(
|
||||
self.tmp / "prgs-repo",
|
||||
"https://gitea.prgs.cc/Scaled-Tech-Consulting/mcp-control-plane.git",
|
||||
)
|
||||
self.mdcps = _init_repo(
|
||||
self.tmp / "mdcps-repo",
|
||||
"https://gitea.dadeschools.net/dadeschools/eAgenda.git",
|
||||
)
|
||||
|
||||
def tearDown(self):
|
||||
self._tmp.cleanup()
|
||||
|
||||
def test_each_namespace_binds_its_own_target(self):
|
||||
a = crr.assess_canonical_repository_root(
|
||||
configured_value=self.prgs,
|
||||
source="env",
|
||||
expected_slug="Scaled-Tech-Consulting/mcp-control-plane",
|
||||
process_project_root=self.tmp.as_posix(),
|
||||
)
|
||||
b = crr.assess_canonical_repository_root(
|
||||
configured_value=self.mdcps,
|
||||
source="env",
|
||||
expected_slug="dadeschools/eAgenda",
|
||||
process_project_root=self.tmp.as_posix(),
|
||||
)
|
||||
self.assertEqual(a["canonical_repo_root"], self.prgs)
|
||||
self.assertEqual(b["canonical_repo_root"], self.mdcps)
|
||||
self.assertNotEqual(a["canonical_repo_root"], b["canonical_repo_root"])
|
||||
|
||||
def test_cross_wired_identity_blocks(self):
|
||||
# prgs path claimed under the mdcps identity → forged binding.
|
||||
got = crr.assess_canonical_repository_root(
|
||||
configured_value=self.prgs,
|
||||
source="env",
|
||||
expected_slug="dadeschools/eAgenda",
|
||||
process_project_root=self.tmp.as_posix(),
|
||||
)
|
||||
self.assertTrue(got["block"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config validation of the profile field (AC5: malformed bindings fail closed)
|
||||
# ---------------------------------------------------------------------------
|
||||
class TestConfigValidation(unittest.TestCase):
|
||||
def test_absent_is_allowed(self):
|
||||
# single-repo default: no field configured
|
||||
gitea_config._validate_canonical_repository_root("p", None)
|
||||
|
||||
def test_valid_absolute_path_ok(self):
|
||||
gitea_config._validate_canonical_repository_root(
|
||||
"p", "/Users/x/Development/mcp-control-plane"
|
||||
)
|
||||
|
||||
def test_relative_path_rejected(self):
|
||||
with self.assertRaises(gitea_config.ConfigError):
|
||||
gitea_config._validate_canonical_repository_root(
|
||||
"p", "relative/path"
|
||||
)
|
||||
|
||||
def test_empty_string_rejected(self):
|
||||
with self.assertRaises(gitea_config.ConfigError):
|
||||
gitea_config._validate_canonical_repository_root("p", " ")
|
||||
|
||||
def test_non_string_rejected(self):
|
||||
with self.assertRaises(gitea_config.ConfigError):
|
||||
gitea_config._validate_canonical_repository_root("p", ["/x"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,221 @@
|
||||
"""Issue #706 F1 integration regression (review 457).
|
||||
|
||||
The unit tests in ``test_issue_706_canonical_repository_root.py`` exercise
|
||||
``crr.assess_canonical_repository_root`` / ``session_ctx.seed_session_context_if_unbound``
|
||||
with a *preselected* slug, so they never drive the real end-to-end path that
|
||||
review 457 found broken:
|
||||
|
||||
_seed_session_context -> _trusted_session_repository -> _workspace_repository_slug
|
||||
-> _local_git_remote_url (cwd=PROJECT_ROOT, always Gitea-Tools)
|
||||
|
||||
Before the fix, the session repository identity was pinned from the *install*
|
||||
checkout remote even when a cross-repository ``canonical_repository_root`` was
|
||||
configured to an external repository (e.g. mcp-control-plane). The mutation
|
||||
preflight then derived ``expected_slug`` from that install-derived pin and
|
||||
``_enforce_canonical_repository_root`` failed closed on a self-inflicted
|
||||
identity mismatch, so the stated cross-repo namespaces stayed blocked.
|
||||
|
||||
These tests use two *real* temporary git repositories and drive the live
|
||||
``mcp_server._seed_session_context`` and ``mcp_server._enforce_canonical_repository_root``
|
||||
functions, asserting the session pins the *configured target* identity and that
|
||||
enforcement accepts the target worktree — while every fail-closed property is
|
||||
preserved.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
import mcp_server # noqa: E402 # loads gitea_mcp_server.py into this namespace
|
||||
import canonical_repository_root as crr # noqa: E402
|
||||
import session_context_binding as session_ctx # noqa: E402
|
||||
|
||||
INSTALL_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
|
||||
TARGET_SLUG = "Scaled-Tech-Consulting/mcp-control-plane"
|
||||
TARGET_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/mcp-control-plane.git"
|
||||
OTHER_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/other-repo.git"
|
||||
|
||||
|
||||
def _git(cwd: str, *args: str) -> str:
|
||||
return subprocess.run(
|
||||
["git", "-C", cwd, *args], capture_output=True, text=True, check=True
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
def _init_repo(path: Path, remote_url: str) -> str:
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
_git(str(path), "init", "-q")
|
||||
_git(str(path), "config", "user.email", "[email protected]")
|
||||
_git(str(path), "config", "user.name", "Test")
|
||||
_git(str(path), "remote", "add", "prgs", remote_url)
|
||||
(path / "README.md").write_text("seed\n")
|
||||
_git(str(path), "add", "README.md")
|
||||
_git(str(path), "commit", "-q", "-m", "seed")
|
||||
return os.path.realpath(str(path))
|
||||
|
||||
|
||||
def _add_worktree(repo_root: str, wt: Path, branch: str) -> str:
|
||||
_git(repo_root, "worktree", "add", "-q", "-b", branch, str(wt))
|
||||
return os.path.realpath(str(wt))
|
||||
|
||||
|
||||
def _profile(role: str, *, canonical: str | None = None,
|
||||
allowed=(TARGET_SLUG,)) -> dict:
|
||||
p = {
|
||||
"profile_name": f"mcp-control-plane-{role}",
|
||||
"role": role,
|
||||
"username": "svc",
|
||||
"allowed_operations": ["gitea.read"],
|
||||
"forbidden_operations": [],
|
||||
"allowed_repositories": list(allowed),
|
||||
}
|
||||
if canonical is not None:
|
||||
p["canonical_repository_root"] = canonical
|
||||
return p
|
||||
|
||||
|
||||
class _Base(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
tmp = Path(self._tmp.name)
|
||||
self.install = _init_repo(tmp / "Gitea-Tools", INSTALL_URL)
|
||||
self.target = _init_repo(tmp / "mcp-control-plane", TARGET_URL)
|
||||
(Path(self.target) / "branches").mkdir()
|
||||
self.target_wt = _add_worktree(
|
||||
self.target, Path(self.target) / "branches" / "author-issue-1",
|
||||
"feat/issue-1",
|
||||
)
|
||||
# PROJECT_ROOT and the install git remote are the Gitea-Tools install
|
||||
# checkout — exactly the source that (mis)seeded the session before.
|
||||
self._p_root = mock.patch.object(mcp_server, "PROJECT_ROOT", self.install)
|
||||
self._p_url = mock.patch.object(
|
||||
mcp_server, "_local_git_remote_url", return_value=INSTALL_URL
|
||||
)
|
||||
self._p_root.start()
|
||||
self._p_url.start()
|
||||
session_ctx._reset_session_context_for_testing()
|
||||
|
||||
def tearDown(self):
|
||||
mock.patch.stopall()
|
||||
session_ctx._reset_session_context_for_testing()
|
||||
|
||||
def _seed(self, profile, env=None):
|
||||
session_ctx._reset_session_context_for_testing()
|
||||
with mock.patch.object(mcp_server, "get_profile", return_value=profile), \
|
||||
mock.patch.dict(os.environ, env or {}, clear=False):
|
||||
return mcp_server._seed_session_context(
|
||||
profile=profile, remote="prgs", host="gitea.prgs.cc",
|
||||
identity="svc",
|
||||
)
|
||||
|
||||
def _enforce(self, profile, env=None):
|
||||
with mock.patch.object(mcp_server, "get_profile", return_value=profile), \
|
||||
mock.patch.dict(os.environ, env or {}, clear=False):
|
||||
mcp_server._enforce_canonical_repository_root(
|
||||
self.target_wt, remote="prgs"
|
||||
)
|
||||
|
||||
|
||||
class TestF1SeedsConfiguredTargetIdentity(_Base):
|
||||
def test_env_config_seeds_target_not_install(self):
|
||||
ctx = self._seed(
|
||||
_profile("reviewer"),
|
||||
env={crr.CANONICAL_ROOT_ENV: self.target},
|
||||
)
|
||||
self.assertEqual(ctx["org"], "Scaled-Tech-Consulting")
|
||||
self.assertEqual(ctx["repository"], "mcp-control-plane")
|
||||
# Regression guard: must NOT be the install repo.
|
||||
self.assertNotEqual(ctx["repository"], "Gitea-Tools")
|
||||
|
||||
def test_profile_field_seeds_target_not_install(self):
|
||||
ctx = self._seed(_profile("merger", canonical=self.target))
|
||||
self.assertEqual(ctx["org"], "Scaled-Tech-Consulting")
|
||||
self.assertEqual(ctx["repository"], "mcp-control-plane")
|
||||
self.assertNotEqual(ctx["repository"], "Gitea-Tools")
|
||||
|
||||
def test_enforce_accepts_target_after_seed_reviewer(self):
|
||||
prof = _profile("reviewer", canonical=self.target)
|
||||
self._seed(prof)
|
||||
# Would raise RuntimeError on the self-inflicted identity mismatch
|
||||
# before the fix.
|
||||
self._enforce(prof)
|
||||
|
||||
def test_enforce_accepts_target_after_seed_merger(self):
|
||||
prof = _profile("merger", canonical=self.target)
|
||||
self._seed(prof)
|
||||
self._enforce(prof)
|
||||
|
||||
def test_env_overrides_profile_for_seed(self):
|
||||
# profile points at install; env points at the real target → env wins.
|
||||
prof = _profile("reviewer", canonical=self.install,
|
||||
allowed=(TARGET_SLUG,))
|
||||
ctx = self._seed(prof, env={crr.CANONICAL_ROOT_ENV: self.target})
|
||||
self.assertEqual(ctx["repository"], "mcp-control-plane")
|
||||
|
||||
|
||||
class TestUnconfiguredDefaultUnchanged(_Base):
|
||||
def test_unconfigured_keeps_install_identity(self):
|
||||
prof = _profile("author", allowed=("Scaled-Tech-Consulting/Gitea-Tools",))
|
||||
ctx = self._seed(prof) # no env, no profile canonical field
|
||||
self.assertEqual(ctx["repository"], "Gitea-Tools")
|
||||
self.assertEqual(ctx["org"], "Scaled-Tech-Consulting")
|
||||
|
||||
|
||||
class TestFailClosed(_Base):
|
||||
def _trusted(self, profile, env=None, *, for_mutation=True):
|
||||
with mock.patch.object(mcp_server, "get_profile", return_value=profile), \
|
||||
mock.patch.dict(os.environ, env or {}, clear=False):
|
||||
return mcp_server._trusted_session_repository(
|
||||
profile, "prgs", for_mutation=for_mutation
|
||||
)
|
||||
|
||||
def test_nonexistent_configured_root_fails_closed(self):
|
||||
res = self._trusted(
|
||||
_profile("reviewer"),
|
||||
env={crr.CANONICAL_ROOT_ENV: self.target + "-missing"},
|
||||
)
|
||||
self.assertIsNone(res["repository"])
|
||||
self.assertTrue(res["reasons"])
|
||||
|
||||
def test_non_git_configured_root_fails_closed(self):
|
||||
plain = Path(self._tmp.name) / "plain"
|
||||
plain.mkdir()
|
||||
res = self._trusted(
|
||||
_profile("reviewer"), env={crr.CANONICAL_ROOT_ENV: str(plain)}
|
||||
)
|
||||
self.assertIsNone(res["repository"])
|
||||
self.assertTrue(any("git" in r for r in res["reasons"]))
|
||||
|
||||
def test_unallowlisted_target_identity_fails_closed(self):
|
||||
# Configured target is valid, but the profile does not authorize it.
|
||||
res = self._trusted(
|
||||
_profile("reviewer", canonical=self.target,
|
||||
allowed=("Scaled-Tech-Consulting/Gitea-Tools",)),
|
||||
)
|
||||
self.assertIsNone(res["repository"])
|
||||
self.assertTrue(any("scope" in r.lower() for r in res["reasons"]))
|
||||
|
||||
def test_forged_identity_conflict_fails_closed_at_enforce(self):
|
||||
# Seed the target, then present a *different* configured root at
|
||||
# enforcement time: the session pin no longer matches → fail closed.
|
||||
other = _init_repo(Path(self._tmp.name) / "other", OTHER_URL)
|
||||
prof_seed = _profile("reviewer", canonical=self.target)
|
||||
self._seed(prof_seed)
|
||||
prof_drift = _profile(
|
||||
"reviewer", canonical=other,
|
||||
allowed=(TARGET_SLUG, "Scaled-Tech-Consulting/other-repo"),
|
||||
)
|
||||
with self.assertRaises(RuntimeError):
|
||||
self._enforce(prof_drift)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,956 @@
|
||||
import sys as _sys
|
||||
from pathlib import Path as _Path
|
||||
_sys.path.insert(0, str(_Path(__file__).resolve().parent))
|
||||
from mutation_profile_fixture import shared_mutation_env # noqa: E402,F401
|
||||
"""Acquired merger-lease provenance + owner finalization (#742).
|
||||
|
||||
Covers the two confirmed defects behind the PR #740 failure:
|
||||
|
||||
1. a lease minted by ``gitea_acquire_merger_pr_lease`` was listed as a
|
||||
sanctioned provenance source but rejected by ``is_sanctioned_session_lease``
|
||||
and reported as unsanctioned by ``describe_session_lease_proof``, so the
|
||||
merge gate refused the merger's own freshly acquired lease;
|
||||
2. no merger-role owner-session operation existed to terminally release or
|
||||
abandon that lease, leaving natural expiry as the only exit.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from datetime import datetime, timedelta, 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 # noqa: E402
|
||||
import merger_lease_adoption as mla # noqa: E402
|
||||
import pr_work_lease as pwl # noqa: E402
|
||||
import reviewer_pr_lease as leases # noqa: E402
|
||||
import task_capability_map as tcm # noqa: E402
|
||||
|
||||
HEAD = "c" * 40
|
||||
OTHER_HEAD = "d" * 40
|
||||
REPO = "Scaled-Tech-Consulting/Gitea-Tools"
|
||||
PR = 740
|
||||
|
||||
|
||||
def _lease_comment(
|
||||
pr_number: int = PR,
|
||||
session_id: str = "merger-session",
|
||||
*,
|
||||
phase: str = "claimed",
|
||||
profile: str = "prgs-merger",
|
||||
identity: str = "merger-user",
|
||||
candidate_head: str = HEAD,
|
||||
comment_id: int = 12354,
|
||||
) -> dict:
|
||||
body = leases.format_lease_body(
|
||||
repo=REPO,
|
||||
pr_number=pr_number,
|
||||
issue_number=742,
|
||||
reviewer_identity=identity,
|
||||
profile=profile,
|
||||
session_id=session_id,
|
||||
worktree="branches/merger-pr740",
|
||||
phase=phase,
|
||||
candidate_head=candidate_head,
|
||||
target_branch="master",
|
||||
target_branch_sha="e" * 40,
|
||||
last_activity=datetime.now(timezone.utc),
|
||||
)
|
||||
return {"id": comment_id, "body": body, "user": {"login": identity}}
|
||||
|
||||
|
||||
def _acquired_session(**overrides) -> dict:
|
||||
session = {
|
||||
"pr_number": PR,
|
||||
"issue_number": 742,
|
||||
"session_id": "merger-session",
|
||||
"reviewer_identity": "merger-user",
|
||||
"profile": "prgs-merger",
|
||||
"worktree": "branches/merger-pr740",
|
||||
"phase": "claimed",
|
||||
"candidate_head": HEAD,
|
||||
"target_branch": "master",
|
||||
"target_branch_sha": "e" * 40,
|
||||
"repo": REPO,
|
||||
"comment_id": 12354,
|
||||
}
|
||||
session.update(overrides)
|
||||
return session
|
||||
|
||||
|
||||
def _record_acquired(session: dict | None = None, **provenance_kwargs) -> dict:
|
||||
provenance = mla.build_lease_provenance(
|
||||
source=mla.SOURCE_ACQUIRE_MERGER,
|
||||
comment_id=provenance_kwargs.pop("comment_id", 12354),
|
||||
**provenance_kwargs,
|
||||
)
|
||||
return leases.record_session_lease(
|
||||
session if session is not None else _acquired_session(),
|
||||
lease_provenance=provenance,
|
||||
)
|
||||
|
||||
|
||||
class TestAcquiredMergerProvenance(unittest.TestCase):
|
||||
"""AC1/AC2/AC4/AC5/AC6 — provenance sanctioning and proof description."""
|
||||
|
||||
def setUp(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def tearDown(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def test_acquired_merger_lease_is_sanctioned(self):
|
||||
session = _record_acquired()
|
||||
self.assertTrue(mla.is_sanctioned_session_lease(session))
|
||||
self.assertEqual(mla.assess_acquired_merger_lease_integrity(session), [])
|
||||
|
||||
def test_proof_description_is_explicit_acquired_merger_kind(self):
|
||||
session = _record_acquired()
|
||||
proof = mla.describe_session_lease_proof(session)
|
||||
self.assertEqual(proof["lease_proof_source"], mla.SOURCE_ACQUIRE_MERGER)
|
||||
self.assertEqual(proof["lease_proof_kind"], "sanctioned_acquire_merger")
|
||||
self.assertTrue(proof["lease_proof_sanctioned"])
|
||||
self.assertEqual(proof["lease_proof_comment_id"], 12354)
|
||||
|
||||
def test_manual_session_seed_without_provenance_still_rejected(self):
|
||||
session = leases.record_session_lease(_acquired_session())
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
proof = mla.describe_session_lease_proof(session)
|
||||
self.assertEqual(proof["lease_proof_kind"], "unsanctioned_manual_seed")
|
||||
|
||||
def test_unknown_provenance_still_rejected(self):
|
||||
session = leases.record_session_lease(
|
||||
_acquired_session(),
|
||||
lease_provenance={"source": "totally_made_up_tool", "comment_id": 1},
|
||||
)
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
self.assertEqual(
|
||||
mla.describe_session_lease_proof(session)["lease_proof_kind"],
|
||||
"unsanctioned",
|
||||
)
|
||||
|
||||
def test_forged_acquired_provenance_without_comment_marker_rejected(self):
|
||||
session = leases.record_session_lease(
|
||||
_acquired_session(comment_id=None),
|
||||
lease_provenance={"source": mla.SOURCE_ACQUIRE_MERGER},
|
||||
)
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
self.assertTrue(
|
||||
any(
|
||||
"comment marker" in reason
|
||||
for reason in mla.assess_acquired_merger_lease_integrity(session)
|
||||
)
|
||||
)
|
||||
|
||||
def test_comment_id_mismatch_between_provenance_and_session_rejected(self):
|
||||
session = _record_acquired(_acquired_session(comment_id=999))
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
|
||||
def test_reviewer_profile_lease_is_not_acquired_merger_proof(self):
|
||||
session = _record_acquired(_acquired_session(profile="prgs-reviewer"))
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
self.assertTrue(
|
||||
any(
|
||||
"not a merger profile" in reason
|
||||
for reason in mla.assess_acquired_merger_lease_integrity(session)
|
||||
)
|
||||
)
|
||||
|
||||
def test_incomplete_fields_fail_closed(self):
|
||||
for field, value in (
|
||||
("session_id", ""),
|
||||
("reviewer_identity", ""),
|
||||
("profile", ""),
|
||||
("repo", ""),
|
||||
("pr_number", None),
|
||||
("candidate_head", None),
|
||||
("candidate_head", "not-a-sha"),
|
||||
):
|
||||
with self.subTest(field=field, value=value):
|
||||
leases.clear_session_lease()
|
||||
session = _record_acquired(_acquired_session(**{field: value}))
|
||||
self.assertFalse(
|
||||
mla.is_sanctioned_session_lease(session),
|
||||
f"{field}={value!r} must not be sanctioned",
|
||||
)
|
||||
|
||||
def test_existing_reviewer_acquire_and_heartbeat_kinds_unchanged(self):
|
||||
for source, kind in (
|
||||
(mla.SOURCE_ACQUIRE, "sanctioned_acquire"),
|
||||
(mla.SOURCE_HEARTBEAT, "sanctioned_heartbeat"),
|
||||
):
|
||||
with self.subTest(source=source):
|
||||
leases.clear_session_lease()
|
||||
session = leases.record_session_lease(
|
||||
{
|
||||
"pr_number": PR,
|
||||
"session_id": "reviewer-session",
|
||||
"candidate_head": HEAD,
|
||||
"comment_id": 101,
|
||||
},
|
||||
lease_provenance=mla.build_lease_provenance(
|
||||
source=source, comment_id=101
|
||||
),
|
||||
)
|
||||
self.assertTrue(mla.is_sanctioned_session_lease(session))
|
||||
self.assertEqual(
|
||||
mla.describe_session_lease_proof(session)["lease_proof_kind"],
|
||||
kind,
|
||||
)
|
||||
|
||||
def test_adoption_provenance_unchanged(self):
|
||||
session = leases.record_session_lease(
|
||||
{
|
||||
"pr_number": PR,
|
||||
"session_id": "merger-session",
|
||||
"candidate_head": HEAD,
|
||||
"comment_id": 202,
|
||||
},
|
||||
lease_provenance=mla.build_lease_provenance(
|
||||
source=mla.SOURCE_ADOPT,
|
||||
comment_id=202,
|
||||
adopted_from_session_id="reviewer-session",
|
||||
),
|
||||
)
|
||||
self.assertTrue(mla.is_sanctioned_session_lease(session))
|
||||
self.assertEqual(
|
||||
mla.describe_session_lease_proof(session)["lease_proof_kind"],
|
||||
"sanctioned_adoption",
|
||||
)
|
||||
|
||||
def test_adoption_without_source_session_still_rejected(self):
|
||||
session = leases.record_session_lease(
|
||||
{"pr_number": PR, "session_id": "merger-session", "comment_id": 202},
|
||||
lease_provenance=mla.build_lease_provenance(
|
||||
source=mla.SOURCE_ADOPT, comment_id=202
|
||||
),
|
||||
)
|
||||
self.assertFalse(mla.is_sanctioned_session_lease(session))
|
||||
|
||||
|
||||
class TestMergeAuthorizationGate(unittest.TestCase):
|
||||
"""AC3 — the merge gate accepts a valid freshly acquired merger lease."""
|
||||
|
||||
def setUp(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def tearDown(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def test_acquired_merger_lease_passes_mutation_lease_gate(self):
|
||||
comments = [_lease_comment()]
|
||||
_record_acquired()
|
||||
result = leases.assess_mutation_lease_gate(
|
||||
pr_number=PR,
|
||||
comments=comments,
|
||||
reviewer_identity="merger-user",
|
||||
session_id="merger-session",
|
||||
mutation="merge",
|
||||
live_head_sha=HEAD,
|
||||
pinned_head_sha=HEAD,
|
||||
)
|
||||
self.assertFalse(result["block"], result["reasons"])
|
||||
|
||||
def test_manual_seed_still_blocked_by_mutation_lease_gate(self):
|
||||
comments = [_lease_comment()]
|
||||
leases.record_session_lease(_acquired_session())
|
||||
result = leases.assess_mutation_lease_gate(
|
||||
pr_number=PR,
|
||||
comments=comments,
|
||||
reviewer_identity="merger-user",
|
||||
session_id="merger-session",
|
||||
mutation="merge",
|
||||
live_head_sha=HEAD,
|
||||
pinned_head_sha=HEAD,
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertTrue(
|
||||
any("sanctioned provenance" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_head_mismatch_still_blocked_with_acquired_lease(self):
|
||||
comments = [_lease_comment()]
|
||||
_record_acquired()
|
||||
result = leases.assess_mutation_lease_gate(
|
||||
pr_number=PR,
|
||||
comments=comments,
|
||||
reviewer_identity="merger-user",
|
||||
session_id="merger-session",
|
||||
mutation="merge",
|
||||
live_head_sha=OTHER_HEAD,
|
||||
pinned_head_sha=HEAD,
|
||||
)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
|
||||
class TestFinalizationAssessment(unittest.TestCase):
|
||||
"""AC7/AC8/AC9/AC10 — owner-only, append-only, idempotent finalization."""
|
||||
|
||||
def setUp(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def tearDown(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def _assess(self, comments, session, **overrides):
|
||||
kwargs = {
|
||||
"pr_number": PR,
|
||||
"session": session,
|
||||
"actor_identity": "merger-user",
|
||||
"actor_profile": "prgs-merger",
|
||||
"actor_session_id": "merger-session",
|
||||
"repo": REPO,
|
||||
"worktree": "branches/merger-pr740",
|
||||
"candidate_head": HEAD,
|
||||
"live_head_sha": HEAD,
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return mla.assess_merger_lease_finalization(comments, **kwargs)
|
||||
|
||||
def test_owner_merger_may_release_its_own_acquired_lease(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess([_lease_comment()], session)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
self.assertFalse(result["already_terminal"])
|
||||
self.assertIn(mla.MERGER_FINALIZATION_MARKER, result["finalization_body"])
|
||||
self.assertIn("phase: released", result["finalization_body"])
|
||||
|
||||
def test_abandon_outcome_supported(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment()],
|
||||
session,
|
||||
outcome=mla.OUTCOME_ABANDONED,
|
||||
reason="non-retryable-merge-failure",
|
||||
)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
self.assertIn("phase: abandoned", result["finalization_body"])
|
||||
self.assertIn(
|
||||
"finalization_reason: non-retryable-merge-failure",
|
||||
result["finalization_body"],
|
||||
)
|
||||
|
||||
def test_unknown_outcome_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess([_lease_comment()], session, outcome="deleted")
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
|
||||
def test_foreign_session_release_rejected(self):
|
||||
session = _record_acquired()
|
||||
comments = [_lease_comment(session_id="someone-else")]
|
||||
result = self._assess(comments, session, actor_session_id="someone-else")
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(
|
||||
any("does not match" in r or "owned by" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_active_foreign_lease_on_thread_rejected(self):
|
||||
session = _record_acquired()
|
||||
comments = [
|
||||
_lease_comment(),
|
||||
_lease_comment(session_id="other-merger", comment_id=12400),
|
||||
]
|
||||
result = self._assess(comments, session)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(any("owned by session_id" in r for r in result["reasons"]))
|
||||
|
||||
def test_reviewer_profile_cannot_finalize_merger_lease(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, actor_profile="prgs-reviewer"
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(
|
||||
any("not a merger profile" in r for r in result["reasons"])
|
||||
)
|
||||
|
||||
def test_reviewer_sourced_session_lease_is_not_merger_owned(self):
|
||||
session = leases.record_session_lease(
|
||||
_acquired_session(profile="prgs-reviewer"),
|
||||
lease_provenance=mla.build_lease_provenance(
|
||||
source=mla.SOURCE_ACQUIRE, comment_id=12354
|
||||
),
|
||||
)
|
||||
result = self._assess([_lease_comment(profile="prgs-reviewer")], session)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(
|
||||
any("merger-owned" in r for r in result["reasons"]), result["reasons"]
|
||||
)
|
||||
|
||||
def test_no_session_lease_rejected(self):
|
||||
result = self._assess([_lease_comment()], None)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
|
||||
def test_wrong_pr_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment(pr_number=741, session_id="merger-session")],
|
||||
session,
|
||||
pr_number=741,
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
|
||||
def test_wrong_repository_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, repo="Scaled-Tech-Consulting/Other"
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(any("repository" in r for r in result["reasons"]))
|
||||
|
||||
def test_wrong_head_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, candidate_head=OTHER_HEAD
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
|
||||
def test_wrong_identity_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, actor_identity="someone-else"
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(any("identity" in r for r in result["reasons"]))
|
||||
|
||||
def test_token_fingerprint_mismatch_rejected(self):
|
||||
session = _record_acquired(native_token_fingerprint="fp-acquire")
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, runtime_token_fingerprint="fp-different"
|
||||
)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(any("fingerprint" in r for r in result["reasons"]))
|
||||
|
||||
def test_matching_token_fingerprint_allowed(self):
|
||||
session = _record_acquired(native_token_fingerprint="fp-acquire")
|
||||
result = self._assess(
|
||||
[_lease_comment()], session, runtime_token_fingerprint="fp-acquire"
|
||||
)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
|
||||
def test_missing_lease_marker_on_thread_rejected(self):
|
||||
session = _record_acquired()
|
||||
result = self._assess([_lease_comment(comment_id=99999)], session)
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertTrue(any("lease marker comment" in r for r in result["reasons"]))
|
||||
|
||||
def test_already_terminal_lease_is_idempotent(self):
|
||||
session = _record_acquired()
|
||||
comments = [
|
||||
_lease_comment(),
|
||||
_lease_comment(phase="released", comment_id=12360),
|
||||
]
|
||||
result = self._assess(comments, session)
|
||||
self.assertTrue(result["already_terminal"])
|
||||
self.assertFalse(result["finalize_allowed"])
|
||||
self.assertIsNone(result["finalization_body"])
|
||||
self.assertEqual(result["reasons"], [])
|
||||
|
||||
def test_abandoned_marker_ends_the_lease(self):
|
||||
comments = [
|
||||
_lease_comment(),
|
||||
_lease_comment(phase="abandoned", comment_id=12361),
|
||||
]
|
||||
self.assertIsNone(
|
||||
leases.find_active_reviewer_lease(comments, pr_number=PR)
|
||||
)
|
||||
|
||||
def test_finalization_is_append_only(self):
|
||||
session = _record_acquired()
|
||||
original = _lease_comment()
|
||||
comments = [original]
|
||||
result = self._assess(comments, session)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
# The assessment never mutates or removes prior ledger entries.
|
||||
self.assertEqual(comments, [original])
|
||||
self.assertEqual(result["lease_comment_id"], 12354)
|
||||
|
||||
|
||||
class TestCrossModuleTerminalPhaseAgreement(unittest.TestCase):
|
||||
"""#742 review 460 — reviewer_pr_lease and pr_work_lease must agree.
|
||||
|
||||
Both modules parse the same append-only lease markers. A phase that is
|
||||
terminal in one and active in the other produces two conflicting truths for
|
||||
the same comment, so an abandoned finalization would still read as an active
|
||||
lease to the conflict-fix acquire and PR-sync inventory paths.
|
||||
"""
|
||||
|
||||
TERMINAL_PHASES = ("released", "blocked", "done", "abandoned")
|
||||
NONTERMINAL_PHASES = ("claimed", "validating")
|
||||
|
||||
def _both_active(self, phase: str) -> tuple[bool, bool]:
|
||||
comments = [_lease_comment(phase=phase)]
|
||||
return (
|
||||
leases.find_active_reviewer_lease(comments, pr_number=PR) is not None,
|
||||
pwl.find_active_reviewer_lease(comments, pr_number=PR) is not None,
|
||||
)
|
||||
|
||||
def test_abandoned_marker_is_not_active_in_pr_work_lease(self):
|
||||
_, pwl_active = self._both_active("abandoned")
|
||||
self.assertFalse(pwl_active)
|
||||
|
||||
def test_terminal_phases_agree_across_modules(self):
|
||||
for phase in self.TERMINAL_PHASES:
|
||||
with self.subTest(phase=phase):
|
||||
rpl_active, pwl_active = self._both_active(phase)
|
||||
self.assertFalse(rpl_active)
|
||||
self.assertFalse(pwl_active)
|
||||
self.assertEqual(rpl_active, pwl_active)
|
||||
|
||||
def test_nonterminal_phases_remain_active_in_both_modules(self):
|
||||
for phase in self.NONTERMINAL_PHASES:
|
||||
with self.subTest(phase=phase):
|
||||
rpl_active, pwl_active = self._both_active(phase)
|
||||
self.assertTrue(rpl_active)
|
||||
self.assertTrue(pwl_active)
|
||||
|
||||
def test_terminal_phase_sets_are_mirrored(self):
|
||||
self.assertEqual(
|
||||
set(leases._TERMINAL_PHASES), set(pwl._TERMINAL_REVIEWER_PHASES)
|
||||
)
|
||||
|
||||
def test_default_released_finalization_behavior_unchanged(self):
|
||||
# The default outcome is still 'released', and a released marker ends
|
||||
# the lease for both modules exactly as before this change.
|
||||
leases.clear_session_lease()
|
||||
try:
|
||||
session = _record_acquired()
|
||||
result = mla.assess_merger_lease_finalization(
|
||||
[_lease_comment()],
|
||||
pr_number=PR,
|
||||
session=session,
|
||||
actor_identity="merger-user",
|
||||
actor_profile="prgs-merger",
|
||||
actor_session_id="merger-session",
|
||||
repo=REPO,
|
||||
worktree="branches/merger-pr740",
|
||||
candidate_head=HEAD,
|
||||
live_head_sha=HEAD,
|
||||
)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
self.assertEqual(result["outcome"], mla.OUTCOME_RELEASED)
|
||||
self.assertIn("phase: released", result["finalization_body"])
|
||||
self.assertEqual(
|
||||
result["finalization_reason"],
|
||||
mla.DEFAULT_MERGER_FINALIZATION_REASON,
|
||||
)
|
||||
finally:
|
||||
leases.clear_session_lease()
|
||||
|
||||
def test_finalization_appends_and_never_rewrites_prior_markers(self):
|
||||
claimed = _lease_comment()
|
||||
original_body = claimed["body"]
|
||||
ledger = [claimed]
|
||||
# A terminal marker is appended alongside the original claim; the
|
||||
# earlier marker is left byte-for-byte intact and is never removed.
|
||||
terminal = _lease_comment(phase="abandoned", comment_id=12361)
|
||||
ledger.append(terminal)
|
||||
self.assertEqual(len(ledger), 2)
|
||||
self.assertEqual(ledger[0]["body"], original_body)
|
||||
self.assertEqual(ledger[0]["id"], 12354)
|
||||
# Newest-wins (#577): the appended terminal marker ends the lease.
|
||||
self.assertIsNone(leases.find_active_reviewer_lease(ledger, pr_number=PR))
|
||||
|
||||
def test_full_ledger_terminal_phases_end_the_lease_in_both_modules(self):
|
||||
for phase in self.TERMINAL_PHASES:
|
||||
with self.subTest(phase=phase):
|
||||
ledger = [
|
||||
_lease_comment(),
|
||||
_lease_comment(phase=phase, comment_id=12361),
|
||||
]
|
||||
for module in (leases, pwl):
|
||||
self.assertIsNone(
|
||||
module.find_active_reviewer_lease(ledger, pr_number=PR),
|
||||
f"{module.__name__} still active after {phase}",
|
||||
)
|
||||
|
||||
|
||||
class TestFullLedgerNewestWins(unittest.TestCase):
|
||||
"""#742 second remediation — chain-scoped newest-wins in pr_work_lease.
|
||||
|
||||
On a realistic append-only ledger (claim -> heartbeat -> terminal),
|
||||
pr_work_lease.find_active_reviewer_lease skipped the terminal marker and
|
||||
walked backward to the older claim of that same chain, resurrecting it. A
|
||||
terminal marker must end its own chain, while a foreign, forged, or
|
||||
malformed terminal marker must never cancel someone else's active lease.
|
||||
"""
|
||||
|
||||
OTHER_SESSION = "other-session"
|
||||
|
||||
def _claim(self, **overrides):
|
||||
return _lease_comment(**overrides)
|
||||
|
||||
def _terminal(self, phase="released", comment_id=12361, **overrides):
|
||||
return _lease_comment(phase=phase, comment_id=comment_id, **overrides)
|
||||
|
||||
def _pwl_active(self, ledger):
|
||||
return pwl.find_active_reviewer_lease(ledger, pr_number=PR)
|
||||
|
||||
# --- the chain must end -------------------------------------------------
|
||||
|
||||
def test_claim_then_each_terminal_phase_leaves_no_active_lease(self):
|
||||
for phase in ("released", "blocked", "done", "abandoned"):
|
||||
with self.subTest(phase=phase):
|
||||
self.assertIsNone(
|
||||
self._pwl_active([self._claim(), self._terminal(phase=phase)])
|
||||
)
|
||||
|
||||
def test_claim_heartbeat_terminal_leaves_no_active_lease(self):
|
||||
for phase in ("released", "blocked", "done", "abandoned"):
|
||||
with self.subTest(phase=phase):
|
||||
ledger = [
|
||||
self._claim(),
|
||||
self._lease_validating(),
|
||||
self._terminal(phase=phase, comment_id=12362),
|
||||
]
|
||||
self.assertIsNone(self._pwl_active(ledger))
|
||||
self.assertIsNone(
|
||||
leases.find_active_reviewer_lease(ledger, pr_number=PR)
|
||||
)
|
||||
|
||||
def _lease_validating(self):
|
||||
return _lease_comment(phase="validating", comment_id=12355)
|
||||
|
||||
# --- foreign / forged terminal markers must not cancel ------------------
|
||||
|
||||
def test_foreign_session_terminal_does_not_cancel_active_claim(self):
|
||||
ledger = [
|
||||
self._claim(),
|
||||
self._terminal(session_id=self.OTHER_SESSION),
|
||||
]
|
||||
active = self._pwl_active(ledger)
|
||||
self.assertIsNotNone(active)
|
||||
self.assertEqual(active["session_id"], "merger-session")
|
||||
|
||||
def test_mismatched_chain_fields_do_not_cancel_active_claim(self):
|
||||
cases = {
|
||||
"identity": {"identity": "someone-else"},
|
||||
"profile": {"profile": "prgs-reviewer"},
|
||||
"candidate_head": {"candidate_head": OTHER_HEAD},
|
||||
}
|
||||
for label, override in cases.items():
|
||||
with self.subTest(field=label):
|
||||
ledger = [self._claim(), self._terminal(**override)]
|
||||
self.assertIsNotNone(
|
||||
self._pwl_active(ledger),
|
||||
f"terminal with wrong {label} must not cancel the claim",
|
||||
)
|
||||
|
||||
def test_terminal_for_another_pr_does_not_cancel_active_claim(self):
|
||||
ledger = [
|
||||
self._claim(),
|
||||
self._terminal(pr_number=741, comment_id=12363),
|
||||
]
|
||||
self.assertIsNotNone(self._pwl_active(ledger))
|
||||
|
||||
def test_terminal_for_another_repository_does_not_cancel_active_claim(self):
|
||||
foreign = self._terminal()
|
||||
foreign["body"] = foreign["body"].replace(
|
||||
REPO, "Scaled-Tech-Consulting/Other-Repo"
|
||||
)
|
||||
self.assertIsNotNone(self._pwl_active([self._claim(), foreign]))
|
||||
|
||||
def test_malformed_terminal_marker_does_not_hide_active_claim(self):
|
||||
malformed = self._terminal()
|
||||
# Strip the session id line: no provable chain, so it cancels nothing.
|
||||
malformed["body"] = "\n".join(
|
||||
line
|
||||
for line in malformed["body"].splitlines()
|
||||
if not line.startswith("session_id:")
|
||||
)
|
||||
active = self._pwl_active([self._claim(), malformed])
|
||||
self.assertIsNotNone(active)
|
||||
self.assertEqual(active["session_id"], "merger-session")
|
||||
|
||||
# --- multi-chain ledgers ------------------------------------------------
|
||||
|
||||
def test_newer_active_chain_survives_older_terminated_chain(self):
|
||||
ledger = [
|
||||
self._claim(),
|
||||
self._terminal(comment_id=12361),
|
||||
_lease_comment(session_id=self.OTHER_SESSION, comment_id=12370),
|
||||
]
|
||||
active = self._pwl_active(ledger)
|
||||
self.assertIsNotNone(active)
|
||||
self.assertEqual(active["session_id"], self.OTHER_SESSION)
|
||||
|
||||
def test_newest_valid_chain_selected_across_multiple_histories(self):
|
||||
ledger = [
|
||||
_lease_comment(session_id="chain-1", comment_id=12300),
|
||||
_lease_comment(session_id="chain-1", phase="released", comment_id=12301),
|
||||
_lease_comment(session_id="chain-2", comment_id=12310),
|
||||
_lease_comment(session_id="chain-2", phase="abandoned", comment_id=12311),
|
||||
_lease_comment(session_id="chain-3", comment_id=12320),
|
||||
]
|
||||
active = self._pwl_active(ledger)
|
||||
self.assertIsNotNone(active)
|
||||
self.assertEqual(active["session_id"], "chain-3")
|
||||
|
||||
def test_all_chains_terminated_leaves_no_active_lease(self):
|
||||
ledger = [
|
||||
_lease_comment(session_id="chain-1", comment_id=12300),
|
||||
_lease_comment(session_id="chain-1", phase="released", comment_id=12301),
|
||||
_lease_comment(session_id="chain-2", comment_id=12310),
|
||||
_lease_comment(session_id="chain-2", phase="abandoned", comment_id=12311),
|
||||
]
|
||||
self.assertIsNone(self._pwl_active(ledger))
|
||||
|
||||
# --- no regression in existing behavior ---------------------------------
|
||||
|
||||
def test_single_marker_behavior_matches_reviewer_pr_lease(self):
|
||||
for phase in (
|
||||
"claimed", "validating", "approved", "merging",
|
||||
"released", "blocked", "done", "abandoned",
|
||||
):
|
||||
with self.subTest(phase=phase):
|
||||
ledger = [_lease_comment(phase=phase)]
|
||||
self.assertEqual(
|
||||
pwl.find_active_reviewer_lease(ledger, pr_number=PR) is not None,
|
||||
leases.find_active_reviewer_lease(ledger, pr_number=PR)
|
||||
is not None,
|
||||
)
|
||||
|
||||
def test_expired_claim_stays_inactive_and_freshness_unchanged(self):
|
||||
past = datetime.now(timezone.utc) - timedelta(hours=4)
|
||||
expired = {
|
||||
"id": 12399,
|
||||
"user": {"login": "merger-user"},
|
||||
"body": leases.format_lease_body(
|
||||
repo=REPO,
|
||||
pr_number=PR,
|
||||
issue_number=742,
|
||||
reviewer_identity="merger-user",
|
||||
profile="prgs-merger",
|
||||
session_id="expired-session",
|
||||
worktree="branches/merger-pr740",
|
||||
phase="claimed",
|
||||
candidate_head=HEAD,
|
||||
target_branch="master",
|
||||
target_branch_sha="e" * 40,
|
||||
last_activity=past,
|
||||
expires_at=past,
|
||||
),
|
||||
}
|
||||
self.assertIsNone(self._pwl_active([expired]))
|
||||
self.assertIsNone(
|
||||
leases.find_active_reviewer_lease([expired], pr_number=PR)
|
||||
)
|
||||
|
||||
def test_active_claim_without_any_terminal_marker_remains_active(self):
|
||||
self.assertIsNotNone(self._pwl_active([self._claim()]))
|
||||
|
||||
def test_production_readers_agree_after_release_tool_finalization(self):
|
||||
"""The ledger the release tool actually writes must read as terminal."""
|
||||
leases.clear_session_lease()
|
||||
try:
|
||||
session = _record_acquired()
|
||||
result = mla.assess_merger_lease_finalization(
|
||||
[_lease_comment()],
|
||||
pr_number=PR,
|
||||
session=session,
|
||||
actor_identity="merger-user",
|
||||
actor_profile="prgs-merger",
|
||||
actor_session_id="merger-session",
|
||||
repo=REPO,
|
||||
worktree="branches/merger-pr740",
|
||||
candidate_head=HEAD,
|
||||
live_head_sha=HEAD,
|
||||
outcome=mla.OUTCOME_ABANDONED,
|
||||
)
|
||||
self.assertTrue(result["finalize_allowed"], result["reasons"])
|
||||
posted = {
|
||||
"id": 12370,
|
||||
"user": {"login": "merger-user"},
|
||||
"body": result["finalization_body"],
|
||||
}
|
||||
ledger = [_lease_comment(), posted]
|
||||
for module in (leases, pwl):
|
||||
with self.subTest(module=module.__name__):
|
||||
self.assertIsNone(
|
||||
module.find_active_reviewer_lease(ledger, pr_number=PR)
|
||||
)
|
||||
# Append-only: the claim marker is still present and unmodified.
|
||||
self.assertEqual(len(ledger), 2)
|
||||
self.assertEqual(ledger[0]["id"], 12354)
|
||||
finally:
|
||||
leases.clear_session_lease()
|
||||
|
||||
|
||||
class TestReleaseMergerLeaseTool(unittest.TestCase):
|
||||
"""AC7/AC9/AC10/AC11 — the native tool contract end to end."""
|
||||
|
||||
def setUp(self):
|
||||
mcp_server._preflight_whoami_called = True
|
||||
mcp_server._preflight_capability_called = True
|
||||
mcp_server._preflight_resolved_role = "merger"
|
||||
mcp_server._preflight_resolved_task = "release_merger_pr_lease"
|
||||
leases.clear_session_lease()
|
||||
|
||||
def tearDown(self):
|
||||
leases.clear_session_lease()
|
||||
|
||||
def _merger_profile(self, **extra):
|
||||
p = {
|
||||
"username": "merger-user",
|
||||
"profile_name": "prgs-merger",
|
||||
"role": "merger",
|
||||
"allowed_operations": [
|
||||
"gitea.read",
|
||||
"gitea.pr.comment",
|
||||
"gitea.pr.merge",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve",
|
||||
"gitea.pr.review",
|
||||
"gitea.pr.request_changes",
|
||||
],
|
||||
}
|
||||
p.update(extra)
|
||||
return p
|
||||
|
||||
def _reviewer_profile(self):
|
||||
return {
|
||||
"username": "reviewer-user",
|
||||
"profile_name": "prgs-reviewer",
|
||||
"role": "reviewer",
|
||||
"allowed_operations": [
|
||||
"gitea.read",
|
||||
"gitea.pr.comment",
|
||||
"gitea.pr.review",
|
||||
"gitea.pr.approve",
|
||||
],
|
||||
"forbidden_operations": ["gitea.pr.merge"],
|
||||
}
|
||||
|
||||
def _call(self, **kwargs):
|
||||
params = {
|
||||
"pr_number": PR,
|
||||
"worktree": "branches/merger-pr740",
|
||||
"candidate_head": HEAD,
|
||||
"remote": "prgs",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
}
|
||||
params.update(kwargs)
|
||||
return mcp_server.gitea_release_merger_pr_lease(**params)
|
||||
|
||||
@patch("gitea_mcp_server.api_request")
|
||||
@patch("gitea_mcp_server._authenticated_username", return_value="merger-user")
|
||||
@patch("gitea_mcp_server._auth", return_value="token pass")
|
||||
@patch(
|
||||
"gitea_mcp_server._resolve",
|
||||
return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"),
|
||||
)
|
||||
@patch("gitea_mcp_server.get_profile")
|
||||
@patch("gitea_mcp_server._fetch_pr_comments")
|
||||
@patch("gitea_mcp_server._verify_role_mutation_workspace")
|
||||
def test_merger_releases_own_acquired_lease(
|
||||
self, _verify, mock_comments, mock_profile, _resolve, _auth, _user, mock_api
|
||||
):
|
||||
mock_profile.return_value = self._merger_profile()
|
||||
mock_comments.return_value = [_lease_comment()]
|
||||
mock_api.side_effect = [
|
||||
{"state": "open", "head": {"sha": HEAD}},
|
||||
{"id": 12370},
|
||||
]
|
||||
_record_acquired()
|
||||
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
|
||||
res = self._call()
|
||||
self.assertTrue(res["success"], res)
|
||||
self.assertTrue(res["finalized"])
|
||||
self.assertEqual(res["finalization_comment_id"], 12370)
|
||||
self.assertEqual(res["outcome"], mla.OUTCOME_RELEASED)
|
||||
# AC11: no active in-session lease survives finalization.
|
||||
self.assertIsNone(leases.get_session_lease())
|
||||
|
||||
@patch("gitea_mcp_server.api_request")
|
||||
@patch("gitea_mcp_server._authenticated_username", return_value="merger-user")
|
||||
@patch("gitea_mcp_server._auth", return_value="token pass")
|
||||
@patch(
|
||||
"gitea_mcp_server._resolve",
|
||||
return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"),
|
||||
)
|
||||
@patch("gitea_mcp_server.get_profile")
|
||||
@patch("gitea_mcp_server._fetch_pr_comments")
|
||||
@patch("gitea_mcp_server._verify_role_mutation_workspace")
|
||||
def test_merger_cannot_release_foreign_session_lease(
|
||||
self, _verify, mock_comments, mock_profile, _resolve, _auth, _user, mock_api
|
||||
):
|
||||
mock_profile.return_value = self._merger_profile()
|
||||
mock_comments.return_value = [_lease_comment(session_id="other-merger")]
|
||||
mock_api.side_effect = [{"state": "open", "head": {"sha": HEAD}}]
|
||||
_record_acquired()
|
||||
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
|
||||
res = self._call()
|
||||
self.assertFalse(res["success"], res)
|
||||
self.assertFalse(res["finalized"])
|
||||
self.assertTrue(res["reasons"])
|
||||
# Fail closed: no comment POST was attempted.
|
||||
self.assertEqual(mock_api.call_count, 1)
|
||||
self.assertIsNotNone(leases.get_session_lease())
|
||||
|
||||
@patch("gitea_mcp_server.api_request")
|
||||
@patch("gitea_mcp_server._authenticated_username", return_value="merger-user")
|
||||
@patch("gitea_mcp_server._auth", return_value="token pass")
|
||||
@patch(
|
||||
"gitea_mcp_server._resolve",
|
||||
return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"),
|
||||
)
|
||||
@patch("gitea_mcp_server.get_profile")
|
||||
@patch("gitea_mcp_server._fetch_pr_comments")
|
||||
@patch("gitea_mcp_server._verify_role_mutation_workspace")
|
||||
def test_already_terminal_release_is_idempotent(
|
||||
self, _verify, mock_comments, mock_profile, _resolve, _auth, _user, mock_api
|
||||
):
|
||||
mock_profile.return_value = self._merger_profile()
|
||||
mock_comments.return_value = [
|
||||
_lease_comment(),
|
||||
_lease_comment(phase="released", comment_id=12360),
|
||||
]
|
||||
mock_api.side_effect = [{"state": "open", "head": {"sha": HEAD}}]
|
||||
_record_acquired()
|
||||
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
|
||||
res = self._call()
|
||||
self.assertTrue(res["success"], res)
|
||||
self.assertFalse(res["finalized"])
|
||||
self.assertTrue(res["already_terminal"])
|
||||
# No second marker posted.
|
||||
self.assertEqual(mock_api.call_count, 1)
|
||||
self.assertIsNone(leases.get_session_lease())
|
||||
|
||||
@patch("gitea_mcp_server.get_profile")
|
||||
def test_reviewer_profile_cannot_invoke_merger_release(self, mock_profile):
|
||||
mock_profile.return_value = self._reviewer_profile()
|
||||
_record_acquired()
|
||||
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-reviewer"}):
|
||||
res = self._call()
|
||||
self.assertFalse(res["success"], res)
|
||||
self.assertFalse(res["finalized"])
|
||||
self.assertTrue(res["reasons"])
|
||||
# The lease is untouched by a rejected cross-role call.
|
||||
self.assertIsNotNone(leases.get_session_lease())
|
||||
|
||||
|
||||
class TestReleaseMergerLeaseCapability(unittest.TestCase):
|
||||
"""AC9 — capability map binds the task to gitea.pr.comment + merger role."""
|
||||
|
||||
def test_task_and_tool_alias_are_merger_scoped(self):
|
||||
for task in ("release_merger_pr_lease", "gitea_release_merger_pr_lease"):
|
||||
with self.subTest(task=task):
|
||||
self.assertEqual(
|
||||
tcm.required_permission(task), "gitea.pr.comment"
|
||||
)
|
||||
self.assertEqual(tcm.required_role(task), "merger")
|
||||
|
||||
def test_reviewer_release_tool_remains_reviewer_scoped(self):
|
||||
self.assertTrue(hasattr(mcp_server, "gitea_release_reviewer_pr_lease"))
|
||||
self.assertNotEqual(
|
||||
mcp_server.gitea_release_reviewer_pr_lease,
|
||||
mcp_server.gitea_release_merger_pr_lease,
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,280 @@
|
||||
"""Regression matrix: session-bound mutation preflight resolves the workspace repo.
|
||||
|
||||
Root-cause follow-up to #733. ``_resolve`` (the actual API-targeting resolver)
|
||||
already prefers the workspace-aligned git remote over the ``REMOTES`` default
|
||||
(bare ``prgs`` → ``Scaled-Tech-Consulting/Timesheet``). The shared #604
|
||||
anti-stomp preflight (``_run_anti_stomp_preflight``) did **not** mirror that: for
|
||||
every mutation task whose call site omitted ``org``/``repo`` it filled the
|
||||
remote-wide ``REMOTES`` default, so the #530 repo guard false-positived
|
||||
``wrong_repo`` against a ``Gitea-Tools`` checkout and blocked native
|
||||
issue/PR/review/merge/lock/cleanup mutations.
|
||||
|
||||
The fix makes ``_run_anti_stomp_preflight`` prefer the trusted, git-remote-
|
||||
derived workspace identity for omitted coordinates on **session-bound** mutation
|
||||
tasks (marking those filled sides explicit, exactly like ``_resolve``), while
|
||||
``delete_branch`` keeps its explicit cross-repository *target* contract (#733)
|
||||
and continues to fail closed on omitted coordinates.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import mcp_server
|
||||
import remote_repo_guard
|
||||
|
||||
# Workspace is bound to Gitea-Tools even though the prgs REMOTES default repo is
|
||||
# Timesheet (the exact cross-repo scenario).
|
||||
LOCAL_GITEA_TOOLS_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
|
||||
|
||||
RECONCILER = {
|
||||
"profile_name": "prgs-reconciler",
|
||||
"role": "reconciler",
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.comment", "gitea.pr.comment",
|
||||
"gitea.pr.close", "gitea.branch.delete",
|
||||
],
|
||||
"forbidden_operations": ["gitea.pr.create", "gitea.pr.merge"],
|
||||
"audit_label": "prgs-reconciler",
|
||||
}
|
||||
|
||||
# Session-bound mutation tasks (every MUTATION_TASK except the explicit-target
|
||||
# delete_branch) must auto-resolve the workspace repository for omitted coords.
|
||||
SESSION_BOUND_TASKS = [
|
||||
"create_issue",
|
||||
"comment_issue",
|
||||
"close_issue",
|
||||
"mark_issue",
|
||||
"lock_issue",
|
||||
"set_issue_labels",
|
||||
"create_label",
|
||||
"create_pr",
|
||||
"close_pr",
|
||||
"edit_pr",
|
||||
"commit_files",
|
||||
"cleanup_merged_pr_branch",
|
||||
"cleanup_stale_claims",
|
||||
"reconcile_merged_cleanups",
|
||||
"reconcile_already_landed_pr",
|
||||
"reconcile_close_superseded_pr",
|
||||
"post_heartbeat",
|
||||
"review_pr",
|
||||
"submit_pr_review",
|
||||
"merge_pr",
|
||||
]
|
||||
|
||||
|
||||
class _AntiStompResolutionBase(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._remotes = patch.dict(mcp_server.REMOTES, {
|
||||
"prgs": {
|
||||
"host": "gitea.prgs.cc",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Timesheet",
|
||||
},
|
||||
})
|
||||
self._remotes.start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
|
||||
def _capture_resolution(self, task, org, repo, *, local_url=LOCAL_GITEA_TOOLS_URL):
|
||||
"""Drive the live anti-stomp runner and capture the org/repo it resolved
|
||||
and passed to the pure assessor."""
|
||||
passthrough = {
|
||||
"allowed": True, "block": False, "blockers": [], "reasons": [],
|
||||
"exact_next_action": "proceed", "blocker_kind": None, "checks": {},
|
||||
}
|
||||
with patch.dict(
|
||||
os.environ, {"GITEA_TEST_FORCE_ANTI_STOMP": "1"}, clear=False
|
||||
), patch(
|
||||
"mcp_server._local_git_remote_url", return_value=local_url
|
||||
), patch(
|
||||
"mcp_server.get_profile", return_value=RECONCILER
|
||||
), patch.object(
|
||||
mcp_server.anti_stomp_preflight,
|
||||
"assess_anti_stomp_preflight",
|
||||
return_value=passthrough,
|
||||
) as m:
|
||||
mcp_server._run_anti_stomp_preflight(
|
||||
task, remote="prgs", org=org, repo=repo
|
||||
)
|
||||
self.assertTrue(m.called)
|
||||
return m.call_args.kwargs
|
||||
|
||||
|
||||
class TestSessionBoundResolution(_AntiStompResolutionBase):
|
||||
def test_omitted_coords_resolve_workspace_repo_and_mark_explicit(self):
|
||||
for task in SESSION_BOUND_TASKS:
|
||||
with self.subTest(task=task):
|
||||
kw = self._capture_resolution(task, None, None)
|
||||
self.assertEqual(
|
||||
kw["resolved_org"], "Scaled-Tech-Consulting", task
|
||||
)
|
||||
self.assertEqual(kw["resolved_repo"], "Gitea-Tools", task)
|
||||
# Workspace-filled sides are intentional alignment → explicit.
|
||||
self.assertTrue(kw["org_explicit"], task)
|
||||
self.assertTrue(kw["repo_explicit"], task)
|
||||
|
||||
def test_resolved_workspace_repo_passes_the_530_guard(self):
|
||||
kw = self._capture_resolution("create_pr", None, None)
|
||||
assessment = remote_repo_guard.assess_remote_repo_match(
|
||||
remote="prgs",
|
||||
resolved_org=kw["resolved_org"],
|
||||
resolved_repo=kw["resolved_repo"],
|
||||
local_remote_url=LOCAL_GITEA_TOOLS_URL,
|
||||
org_explicit=kw["org_explicit"],
|
||||
repo_explicit=kw["repo_explicit"],
|
||||
)
|
||||
self.assertFalse(assessment["block"], assessment)
|
||||
|
||||
def test_explicit_caller_coords_are_preserved(self):
|
||||
kw = self._capture_resolution(
|
||||
"create_pr", "Scaled-Tech-Consulting", "Gitea-Tools"
|
||||
)
|
||||
self.assertEqual(kw["resolved_org"], "Scaled-Tech-Consulting")
|
||||
self.assertEqual(kw["resolved_repo"], "Gitea-Tools")
|
||||
self.assertTrue(kw["org_explicit"])
|
||||
self.assertTrue(kw["repo_explicit"])
|
||||
|
||||
def test_no_workspace_identity_falls_back_to_remotes_default(self):
|
||||
# No derivable local remote → best-effort REMOTES default preserved,
|
||||
# not marked explicit (so the guard still corroborates when able).
|
||||
kw = self._capture_resolution("create_pr", None, None, local_url=None)
|
||||
self.assertEqual(kw["resolved_repo"], "Timesheet")
|
||||
self.assertFalse(kw["org_explicit"])
|
||||
self.assertFalse(kw["repo_explicit"])
|
||||
|
||||
|
||||
class TestDeleteBranchContractPreserved(_AntiStompResolutionBase):
|
||||
"""delete_branch keeps its explicit-target contract (#733): omitted coords
|
||||
still resolve the remote-wide default and are NOT marked explicit, so the
|
||||
guard fails closed against a mismatched workspace."""
|
||||
|
||||
def test_omitted_coords_still_remote_default_not_explicit(self):
|
||||
kw = self._capture_resolution("delete_branch", None, None)
|
||||
self.assertEqual(kw["resolved_repo"], "Timesheet")
|
||||
self.assertFalse(kw["org_explicit"])
|
||||
self.assertFalse(kw["repo_explicit"])
|
||||
assessment = remote_repo_guard.assess_remote_repo_match(
|
||||
remote="prgs",
|
||||
resolved_org=kw["resolved_org"],
|
||||
resolved_repo=kw["resolved_repo"],
|
||||
local_remote_url=LOCAL_GITEA_TOOLS_URL,
|
||||
org_explicit=kw["org_explicit"],
|
||||
repo_explicit=kw["repo_explicit"],
|
||||
)
|
||||
self.assertTrue(assessment["block"])
|
||||
|
||||
def test_explicit_coords_still_forwarded(self):
|
||||
kw = self._capture_resolution(
|
||||
"delete_branch", "Scaled-Tech-Consulting", "Gitea-Tools"
|
||||
)
|
||||
self.assertEqual(kw["resolved_repo"], "Gitea-Tools")
|
||||
self.assertTrue(kw["org_explicit"])
|
||||
self.assertTrue(kw["repo_explicit"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
class TestAuthorCallSiteForwardsOrgRepo(_AntiStompResolutionBase):
|
||||
"""#735: author mutation entrypoints must pass org/repo into preflight."""
|
||||
|
||||
def _assert_call_forwards(self, fn_name: str, *args, **kwargs):
|
||||
import inspect
|
||||
fn = getattr(mcp_server, fn_name)
|
||||
# Force preflight path to raise with the kwargs it received so we can
|
||||
# prove org/repo were forwarded without performing the full mutation.
|
||||
captured = {}
|
||||
|
||||
def _capture(*a, **kw):
|
||||
captured.update(kw)
|
||||
captured["_args"] = a
|
||||
raise RuntimeError("capture-only")
|
||||
|
||||
with patch.object(mcp_server, "verify_preflight_purity", side_effect=_capture), \
|
||||
patch.object(mcp_server, "get_profile", return_value={
|
||||
"profile_name": "prgs-author",
|
||||
"role": "author",
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.create", "gitea.issue.comment",
|
||||
"gitea.pr.create", "gitea.repo.commit", "gitea.branch.push",
|
||||
],
|
||||
"forbidden_operations": [],
|
||||
"audit_label": "prgs-author",
|
||||
}), patch.object(
|
||||
mcp_server, "_resolve",
|
||||
return_value=("gitea.prgs.cc", "Scaled-Tech-Consulting", "Gitea-Tools"),
|
||||
), patch.object(
|
||||
mcp_server, "_auth", return_value="token fake",
|
||||
), patch.object(
|
||||
mcp_server.role_session_router,
|
||||
"check_author_mutation_after_reviewer_stop",
|
||||
return_value=(True, []),
|
||||
), patch.object(
|
||||
mcp_server, "_namespace_mutation_block", return_value=None
|
||||
), patch.object(
|
||||
mcp_server, "_profile_permission_block", return_value=None
|
||||
), patch.object(
|
||||
mcp_server, "_production_guard_block_from_exc", return_value=None
|
||||
):
|
||||
try:
|
||||
fn(*args, **kwargs)
|
||||
except RuntimeError as exc:
|
||||
if "capture-only" not in str(exc):
|
||||
# Some tools re-raise; still require capture when preflight ran.
|
||||
if not captured:
|
||||
raise
|
||||
self.assertTrue(captured, f"{fn_name} never called verify_preflight_purity")
|
||||
self.assertEqual(captured.get("org"), "Scaled-Tech-Consulting", fn_name)
|
||||
self.assertEqual(captured.get("repo"), "Gitea-Tools", fn_name)
|
||||
|
||||
def test_create_issue_forwards_explicit_org_repo(self):
|
||||
self._assert_call_forwards(
|
||||
"gitea_create_issue",
|
||||
"title",
|
||||
body="body with acceptance criteria for gate",
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
worktree_path="/tmp/fake-wt",
|
||||
)
|
||||
|
||||
def test_create_pr_forwards_explicit_org_repo(self):
|
||||
self._assert_call_forwards(
|
||||
"gitea_create_pr",
|
||||
title="t",
|
||||
head="fix/issue-735-x",
|
||||
base="master",
|
||||
body="Closes #735",
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
worktree_path="/tmp/fake-wt",
|
||||
)
|
||||
|
||||
def test_mark_issue_forwards_explicit_org_repo(self):
|
||||
self._assert_call_forwards(
|
||||
"gitea_mark_issue",
|
||||
issue_number=735,
|
||||
action="start",
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
worktree_path="/tmp/fake-wt",
|
||||
)
|
||||
|
||||
def test_commit_files_forwards_explicit_org_repo(self):
|
||||
self._assert_call_forwards(
|
||||
"gitea_commit_files",
|
||||
files=[{"operation": "update", "path": "x", "content_plain": "y"}],
|
||||
message="m",
|
||||
remote="prgs",
|
||||
org="Scaled-Tech-Consulting",
|
||||
repo="Gitea-Tools",
|
||||
)
|
||||
Reference in New Issue
Block a user