Compare commits

...
Author SHA1 Message Date
sysadminandClaude Opus 4.8 fde95b9266 fix(mcp): chain-scoped newest-wins reviewer lease reads in pr_work_lease (#742)
Second author remediation on PR #743. Fixes the full-ledger defect surfaced by
the first remediation.

Root cause: pr_work_lease.find_active_reviewer_lease iterated the reviewer
markers newest-first and used `continue` on a terminal marker. On a realistic
append-only ledger (claimed -> validating -> terminal) it therefore stepped
over the terminal marker and returned the older claim of the very chain that
marker had just ended. reviewer_pr_lease got strict newest-wins under #577;
pr_work_lease never did, so the two modules disagreed for released, blocked,
done, and abandoned alike, and merger owner finalization did not leave "no
active lease" for pr_work_lease consumers (conflict-fix acquire, PR sync
inventory).

Fix: a claim is skipped when a later marker terminates its own chain. Chain
identity is (repo, pr_number, candidate_head, session_id, reviewer_identity,
profile) via the new _reviewer_chain_key; _chain_terminated_after scans only
markers appended after the candidate. Consequences:

- a valid terminal marker ends its matching earlier claim (no resurrection);
- a foreign-session, wrong-repo, wrong-PR, wrong-head, wrong-identity, or
  wrong-profile terminal marker cannot cancel another session's active lease,
  which strict newest-wins alone would have allowed;
- a malformed terminal marker has no provable chain key, so it cancels nothing
  and cannot hide a valid active claim;
- expiry, freshness, phase sets, ownership and integrity checks, both parsers,
  and find_active_conflict_fix_lease are untouched;
- history stays append-only; no marker is deleted or rewritten.

Reviewer lease markers carry no token field, so token-fingerprint validation
remains where it already lives (session provenance, merger finalization) and is
not weakened here.

Tests: new TestFullLedgerNewestWins in tests/test_merger_lease_finalization.py
covers claimed->released/blocked/done/abandoned, claimed->validating->terminal,
foreign-session and mismatched repo/PR/head/identity/profile terminals, the
malformed terminal marker, a newer active chain surviving an older terminated
chain, newest-valid-chain selection across multiple histories, all-chains-
terminated, single-marker parity between the two modules across eight phases,
expired-claim/freshness non-regression, and the real ledger written by
gitea_release_merger_pr_lease reading as terminal in both modules with the
prior marker preserved. TestCrossModuleTerminalPhaseAgreement's scope-limited
placeholder test is replaced by a real both-modules full-ledger assertion.

Verified 10 of the new tests fail at the prior head 7ae5f3a and pass here.

Validation: focused TestFullLedgerNewestWins 14 passed / 21 subtests;
tests/test_merger_lease_finalization.py 59 passed / 42 subtests; targeted
pr_work_lease + reviewer/merger lease + adoption + provenance + acquire/release
+ anti-stomp + capability-map + decision-lock + report-validator suites 309
passed / 41 subtests; full suite 3326 passed, 6 skipped, 2 failed. Both
failures (test_issue_702_review_findings_f1_f6 F1 recovery, reconciler
supersession close) reproduce identically on clean baseline master a8d2087 and
are pre-existing #737 org/repo-forwarding drift. git diff --check clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-18 10:12:49 -04:00
sysadminandClaude Opus 4.8 7ae5f3a541 fix(mcp): mirror abandoned terminal phase into pr_work_lease (#742 review 460)
Addresses REQUEST_CHANGES review 460 on PR #743 @ 22d0fdd.

reviewer_pr_lease._TERMINAL_PHASES gained "abandoned" for owner-session merger
finalization, but pr_work_lease._TERMINAL_REVIEWER_PHASES did not. The two
modules parse the same append-only lease markers, so the same comment read as
terminal through reviewer_pr_lease and active through pr_work_lease, leaving
the conflict-fix acquire and PR-sync inventory readers with a stale active
lease after an abandoned finalization.

Fix: add "abandoned" to pr_work_lease._TERMINAL_REVIEWER_PHASES, with a comment
recording that the two sets must stay mirrored.

Reproduced before the fix (single abandoned marker):
  reviewer_pr_lease active=False, pr_work_lease active=True
After: both False; released/blocked/done unchanged in both modules.

Tests: new TestCrossModuleTerminalPhaseAgreement in
tests/test_merger_lease_finalization.py proves the abandoned marker is inactive
in pr_work_lease, that both modules agree for released/blocked/done/abandoned,
that claimed/validating stay active in both, that the two phase sets are
mirrored, that the default 'released' finalization outcome and reason are
unchanged, and that finalization appends without rewriting or deleting the
prior marker.

Separately pinned, NOT fixed here: on a claim-then-terminal ledger,
pr_work_lease.find_active_reviewer_lease skips the terminal marker and walks
back to the older claim, so it still reports an active lease. That newest-wins
gap is pre-existing on clean baseline a8d2087 for released, blocked, and done
alike — the #577 fix landed in reviewer_pr_lease only — and is out of scope for
this bounded remediation. test_abandoned_matches_preexisting_terminal_phases_on_full_ledger
pins that "abandoned" introduces no behavior of its own, so all four phases can
be reconciled in one separate change.

Validation: focused TestCrossModuleTerminalPhaseAgreement 7 passed / 9 subtests;
tests/test_merger_lease_finalization.py 45 passed / 20 subtests; related
pr_work_lease + reviewer/merger lease + adoption + provenance + capability-map +
anti-stomp + report-validator suites 261 passed / 41 subtests; full suite 3312
passed, 6 skipped, 2 failed. Both failures (test_issue_702_review_findings_f1_f6
F1 recovery, reconciler supersession close) reproduce identically on clean
baseline master a8d2087 and are pre-existing #737 org/repo-forwarding drift.
git diff --check clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-18 09:53:34 -04:00
sysadminandClaude Opus 4.8 22d0fdd251 fix(mcp): accept acquired merger-lease provenance and add owner finalization (Closes #742)
Root cause (confirmed on PR #740, session 33780-7168cbeeba58, marker 12354):

merger_lease_adoption.SANCTIONED_PROVENANCE_SOURCES already contained
SOURCE_ACQUIRE_MERGER, so the membership check passed, but
is_sanctioned_session_lease() branched only for SOURCE_ADOPT and for
{SOURCE_ACQUIRE, SOURCE_HEARTBEAT} and fell through to `return False` for a
lease minted by gitea_acquire_merger_pr_lease. assess_mutation_lease_gate()
therefore appended "in-session lease lacks sanctioned provenance" and
gitea_merge_pr fail-closed on the merger's own freshly acquired lease.
describe_session_lease_proof() likewise had no branch for that source and
reported the lease as lease_proof_kind=unsanctioned. Separately, no
merger-role owner-session operation existed to end that lease, so a failed
merger lease could only expire.

A. Acquired merger provenance

is_sanctioned_session_lease() now accepts SOURCE_ACQUIRE_MERGER, but only via
the new assess_acquired_merger_lease_integrity(), which fails closed unless
the in-session record is complete and self-consistent: comment marker present
and matching between provenance and session, non-empty session id, holder
identity, merger profile, repository, valid PR number, and a normalized
40-hex candidate_head. describe_session_lease_proof() reports the explicit
kind sanctioned_acquire_merger. Manual _SESSION_LEASE seeding, forged or
incomplete records, mismatched fields, and unknown provenance all remain
rejected; reviewer-acquire, reviewer-heartbeat, and merger-adoption paths are
untouched.

B. Merger owner-session finalization

New native tool gitea_release_merger_pr_lease terminally releases or abandons
a merger's own comment-backed lease when the merge does not occur. It is
merger-only (gitea.read + gitea.pr.comment + gitea.pr.merge; reviewer profiles
lack pr.merge) with an explicit capability-map task release_merger_pr_lease /
gitea_release_merger_pr_lease bound to gitea.pr.comment + role merger, and it
is listed as role-exclusive in the resolver. assess_merger_lease_finalization()
verifies exact session ownership, repository, PR, candidate head vs live head,
profile, identity, marker presence on the thread, and the native runtime token
fingerprint recorded at acquisition; foreign-session release, reviewer-profile
use, and any mismatch fail closed. Finalization appends a terminal lease marker
and never edits or deletes ledger history; an already-terminal lease returns
already_terminal and posts nothing. The PR, approval, decision lock, branch,
and worktree are all preserved. gitea_release_reviewer_pr_lease is unchanged
and is not repurposed for merger sessions.

"abandoned" is added to reviewer_pr_lease._TERMINAL_PHASES; without it an
abandoned marker would fall through the generic non-empty phase branch and
keep re-arming the lease as active.

Tests: new tests/test_merger_lease_finalization.py (38 tests, 11 subtests)
covers all twelve acceptance criteria — provenance sanctioning, explicit proof
kind, merge-gate acceptance, manual-seed and unknown-provenance rejection,
profile/role/session/head/repository/fingerprint mismatches, owner release,
foreign-session refusal, reviewer refusal, append-only idempotent
finalization, no surviving lease after finalization, and no regression in
adoption or reviewer-lease behavior. The defect was reproduced on clean
baseline a8d2087 first (is_sanctioned=False, proof_kind=unsanctioned, no
finalization path).

Validation: targeted lease/capability suites 272 passed. Full suite
3305 passed, 6 skipped, 2 failed — both failures
(test_issue_702_review_findings_f1_f6 F1 recovery, reconciler supersession
close) reproduce identically on clean baseline master a8d2087 in a throwaway
worktree and are pre-existing #737 org/repo-forwarding drift, unrelated to
this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-18 09:33:58 -04:00
sysadmin a8d2087b4a Merge pull request 'feat(mcp): immutable canonical_repository_root for cross-repo namespaces (Closes #706)' (#736) from feat/issue-706-canonical-repository-root into master 2026-07-18 01:36:03 -05:00
sysadminandClaude Opus 4.8 61c0d73cd1 fix(mcp): seed cross-repo session identity from configured canonical root (#706 F1)
Addresses REQUEST_CHANGES review 457 on PR #736.

Root cause: _trusted_session_repository derived the session org/repository
solely from _workspace_repository_slug -> _local_git_remote_url, which runs
`git remote get-url` in PROJECT_ROOT (the Gitea-Tools install checkout). A
cross-repository namespace with a configured canonical_repository_root then
pinned the Gitea-Tools identity while #274 filesystem membership bound the
external target, so _enforce_canonical_repository_root failed closed on a
self-inflicted identity mismatch and the mcp-control-plane / eagenda
namespaces stayed blocked end-to-end.

Fix: when a canonical_repository_root is configured (env over profile) and
passes existence / git-toplevel / resolvable-remote-identity validation, the
session repository slug is derived from repository_identity_slug(configured
root) instead of the install remote. The derived identity is still authorized
by the profile allowed_repositories allowlist (never self-authorizing); the
canonical filesystem root and org/repo identity remain immutable for the
session; invalid / non-git / unresolvable / unallowlisted / forged / drift
cases fail closed; unconfigured single-repo Gitea-Tools namespaces keep the
install-derived slug unchanged.

Tests: new tests/test_issue_706_f1_seed_identity_integration.py drives the
real _seed_session_context -> _enforce_canonical_repository_root path with two
real temporary git repositories (install=Gitea-Tools, configured target=
mcp-control-plane): env + profile config, reviewer + merger role kinds, and
fail-closed cases (nonexistent / non-git / unallowlisted / forged identity
drift). Reproduces the F1 block before the fix; green after.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-18 01:27:26 -04:00
jcwalker3 6a0d7bbef4 Merge branch 'master' into feat/issue-706-canonical-repository-root 2026-07-17 23:12:55 -05:00
sysadmin 29d96c8946 Merge pull request 'fix(mcp): forward explicit org/repo through author mutation preflight (Closes #735)' (#737) from fix/issue-735-author-org-repo-forwarding into master 2026-07-17 22:50:28 -05:00
sysadminandClaude Opus 4.8 0d8a2c2b1d feat(mcp): immutable canonical_repository_root for cross-repo namespaces (Closes #706)
The MCP server derived the canonical repository root from the install
checkout the server script lives in (PROJECT_ROOT), so any namespace that
ran the server against an external repository (e.g. eagenda-author, or the
mcp-control-plane reviewer/merger namespaces) failed every mutation: the
branches-only / worktree-membership guards (#274) compared the task
workspace against the Gitea-Tools .git directory it can never belong to.

Separate the two concepts:
- PROJECT_ROOT stays the immutable code/install root.
- A new, optional, namespace-scoped canonical_repository_root binds the
  session to the working root of the target repository.

Implementation:
- canonical_repository_root.py: resolve the binding from profile/env
  (GITEA_CANONICAL_REPOSITORY_ROOT overrides the profile field), validate
  existence + git identity, fail closed on missing/conflicting/forged
  bindings. Preserves the single-repo default when unconfigured.
- session_context_binding.py: pin canonical_repository_root into the
  immutable #714 session context; drift fails closed.
- namespace_workspace_binding.py: route the configured target root through
  resolve_namespace_mutation_context so #274 / membership guards evaluate
  against the target repository.
- gitea_mcp_server.py: seed + enforce the binding in the mutation preflight
  (both purity-order and FORCE_PRODUCTION_GUARDS paths); validate repository
  identity and git common-directory membership.
- gitea_config.py: validate the optional absolute-path profile field.
- gitea_auth.py: surface the config-sourced field through get_profile.

No weakening of root-checkout, remote/repository, or anti-stomp guards; the
single-repo Gitea-Tools path is unchanged.

Tests: two distinct real git repositories/worktrees prove cross-repository
bindings resolve, enforce membership, and fail closed on forged/conflicting
identity and simultaneous prgs/mdcps isolation.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-17 23:31:39 -04:00
13 changed files with 2664 additions and 8 deletions
+267
View File
@@ -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,
}
+4
View File
@@ -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,
+27
View File
@@ -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.
+312 -2
View File
@@ -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.
@@ -1115,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(
@@ -1148,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(
@@ -1606,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,
@@ -1644,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,
@@ -1654,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
@@ -11467,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,
@@ -11656,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,
@@ -16364,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",
+350
View File
@@ -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],
*,
+17 -2
View File
@@ -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
View File
@@ -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
+4 -1
View File
@@ -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",
+18
View File
@@ -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"),
}
+11
View File
@@ -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()
+956
View File
@@ -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()