Compare commits

...
Author SHA1 Message Date
sysadminandClaude Opus 4.8 61c3a57df5 fix(mcp): consume canonical target root in cross-repository operations (Closes #741)
#706 introduced the immutable `canonical_repository_root` and #739/#740 routed
three consumption paths through it. The paths #740 left behind all shared one
shape: the *filesystem* guards had been migrated to the canonical root, but
*repository identity* still bottomed out in `_local_git_remote_url`, which ran
`git remote get-url` with `cwd=PROJECT_ROOT` — always the Gitea-Tools install
checkout. The two halves of a single assessment therefore described two
different repositories.

For a namespace bound to another repository this inverts the guards rather than
merely weakening them: an operation naming the genuinely bound target is
rejected, while one naming Gitea-Tools is accepted.

Central helper
--------------
Adds `_canonical_local_git_root()` as the single place a target-repository root
is resolved: the configured canonical root when one is declared, else
`PROJECT_ROOT`. A configured-but-invalid root is never silently replaced by the
install checkout. Single-repository behaviour is byte-for-byte unchanged.

Defects fixed
-------------
* `_local_git_remote_url` now runs in the canonical target root, which corrects
  every downstream identity consumer at once (`_resolve`, the #530 remote/repo
  guard, the anti-stomp org/repo fill, `_workspace_repository_slug`).
* `_verify_role_mutation_workspace` omitted `configured_canonical_root`, so the
  #274 branches-only / worktree-membership guards validated
  `Gitea-Tools/branches/` instead of the bound target. It now threads the root
  exactly as `_resolve_namespace_mutation_context` does.
* `_resolve` derived omitted coordinates by looking a remote up *by name*. A
  target checkout commonly names its remote `origin` rather than `prgs`, so the
  lookup returned None and the coordinates fell through to the remote-wide
  default target — an unrelated repository. It now prefers
  `_canonical_repository_slug`, which probes candidate remote names.
* Explicit `org`/`repo` short-circuit the #530 match check, so caller
  coordinates bypassed validation entirely. They may now only *confirm* a
  canonical binding, never override it, and fail closed in both directions.
* Reconciler ancestry, fetch, worktree-inventory and cleanup call sites, the
  author worktree derivation in `gitea_lock_issue`/`gitea_create_pr`, and the
  `control_clean` porcelain probe now use the canonical target root.
* `gitea_config`: v2-`environments` silently *dropped* `canonical_repository_root`
  during flattening, so such a namespace fell back to the install root — a
  fail-open. It is now validated and propagated. The v1 path validates it too,
  so all three loaders behave identically.

Preserved as installation-scoped
--------------------------------
Server parity (`master_parity_gate`), workflow/schema/skill loading, self-code
hashing and stale-runtime detection, and `mirror_refs.sh` lookup remain anchored
to `PROJECT_ROOT`. The `server_implementation` vs `target_repository` parity
dimensions from #740 stay separately labelled, and only the server dimension
gates mutations.

Tests
-----
`tests/test_issue_741_canonical_root_consumers.py` (51 tests, 52 subtests) drives
a role-by-operation matrix over author/reviewer/merger/reconciler against:
correct cross-repository target, wrong repository, wrong worktree, explicit
matching and mismatched coordinates, missing/invalid canonical root, immutable
first-bind, and request-override attempts — plus both cross-repository
directions and all three config loaders. Real git repositories, no network, no
branch deletion, no merge.

The module reinstalls the genuine `_local_git_remote_url` per test: an autouse
conftest fixture permanently reassigns it to a working-directory-independent
stub, which is precisely the behaviour under test.

Full suite: 3408 passed, 6 skipped, 2 failed. Both failures
(`test_issue_702_review_findings_f1_f6`, `test_reconciler_supersession_close`)
reproduce identically on clean master c908ed6050 and are pre-existing.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AYGdWAwuA6UNDc9c3CGipU
2026-07-18 12:02:31 -04:00
sysadmin c908ed6050 Merge pull request 'fix(mcp): complete canonical-root consumption for cross-repository namespaces (Closes #739)' (#740) from feat/issue-739-canonical-root-consumption into master 2026-07-18 10:34:10 -05:00
sysadmin 0a38d92382 Merge pull request 'fix(mcp): accept acquired merger-lease provenance and add owner finalization (Closes #742)' (#743) from fix/issue-742-merger-lease-provenance into master 2026-07-18 10:12:27 -05:00
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
9 changed files with 2625 additions and 28 deletions
+153
View File
@@ -0,0 +1,153 @@
# Installation root vs canonical target repository root
## Purpose
This document (tracked as issue #741, building on #706 and #739/#740) explains
the two distinct filesystem roots the Gitea-Tools MCP server reasons about, why
conflating them silently targets the wrong repository, and which rule applies
when you add a new consumer.
It is the repository-scope companion to
[`gitea-execution-profiles.md`](gitea-execution-profiles.md) (the profile model)
and [`gitea-dual-namespace-deployment.md`](gitea-dual-namespace-deployment.md)
(the per-role namespace model).
## The two roots
| | Installation root | Canonical target repository root |
|---|---|---|
| What it is | The checkout the server *code* lives in | The working root of the repository whose issues/PRs/branches the namespace *mutates* |
| How it is derived | `PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))` | Configured per namespace, then pinned immutably into the session |
| Configured by | Nothing — it follows the script | `canonical_repository_root` profile field, or the `GITEA_CANONICAL_REPOSITORY_ROOT` environment variable |
| Changes at runtime? | No | No — first bind wins for the life of the process |
| Accessor | `PROJECT_ROOT` | `_canonical_local_git_root()` (filesystem) / `_canonical_repository_slug()` (identity) |
For a **single-repository** namespace — every Gitea-Tools namespace today — the
two roots are the same path, and nothing about the existing behaviour changes.
The distinction only becomes observable once a namespace is pointed at a
different repository.
## Which root does my code need?
Ask what the operation is *about*, not where the file happens to sit.
**Use the installation root (`PROJECT_ROOT`)** when the operation concerns the
Gitea-Tools software itself:
- server implementation / version parity (`master_parity_gate`, the
`startup_head` vs `current_head` staleness gate);
- loading the server's own workflow, schema and skill files;
- self-code hashing and stale-runtime detection;
- locating installed scripts such as `mirror_refs.sh`.
These are intentionally install-scoped. Do not "fix" them.
**Use the canonical target root (`_canonical_local_git_root()`)** when the
operation concerns the repository being worked on:
- `git remote get-url` for repository identity;
- branch creation, push, and commit;
- ancestry and merge-base proofs;
- worktree inventory, cleanup, and branch deletion;
- any local git subprocess whose result feeds a mutation guard.
**If you cannot tell, fail closed.** An ambiguous consumer that guesses the
install root is the exact defect class #741 exists to eliminate.
## Why conflating them inverts the guards
Before #741, `_local_git_remote_url()` ran `git remote get-url` with
`cwd=PROJECT_ROOT` unconditionally. Every consumer of repository *identity*
`_resolve`, the #530 remote/repo guard, the anti-stomp org/repo fill,
`_workspace_repository_slug` — therefore read the Gitea-Tools remote and called
it "the workspace", no matter which repository the namespace was bound to.
For a namespace whose canonical root points elsewhere, this **inverts** the
guard rather than merely weakening it:
- an operation naming the genuinely bound target repository is **rejected**,
because that slug does not appear in the Gitea-Tools remote URL;
- an operation naming Gitea-Tools is **accepted**.
The filesystem guards (#274 branches-only and worktree membership) had already
been migrated to the canonical root by #706, so the two halves of a single
assessment described two different repositories.
A related subtlety: repository identity must not be derived by looking a remote
up by *name*. A target checkout commonly names its remote `origin` rather than
`prgs`, so a name-keyed lookup returns nothing and the omitted coordinates fall
through to the remote-wide default *target* — an unrelated repository.
`_canonical_repository_slug()` probes candidate remote names against the
canonical root instead.
`_canonical_local_git_root()` is now the one place a target root is resolved.
Do not re-derive it; new code that needs a target root calls that helper.
## Configuration
Declare the binding on the profile, alongside `allowed_repositories`:
```json
{
"profiles": {
"example-author": {
"role": "author",
"canonical_repository_root": "/absolute/path/to/target-repo",
"allowed_repositories": ["Example-Org/target-repo"]
}
}
}
```
The namespace-scoped environment variable
`GITEA_CANONICAL_REPOSITORY_ROOT` overrides the profile field, and is normally
exported next to the server `cwd` in the MCP client configuration.
Validation is layered, and each layer fails closed:
1. **Config load.** The path must be a non-empty absolute string. All supported
loaders — v1, v2-`environments`, and v2-`contexts` — validate it identically.
(Before #741 only the v2-`contexts` loader validated it, and
v2-`environments` silently *dropped* the field during flattening, so the
namespace fell back to the install root — a fail-open.)
2. **Bind time.** The path must exist, be a git repository, and resolve to a
repository identity matching the session's authorized slug. A configured but
unresolvable root is never replaced by the install identity.
3. **Every mutation.** The pinned root is compared against the live configured
value; a mismatch is treated as a forged or conflicting binding.
`allowed_repositories` remains a separate authorization boundary (#714): the
canonical root determines *which* repository is derived, and
`allowed_repositories` determines whether the session may act on it. A root that
resolves to a repository outside that list fails closed.
## Explicit coordinates confirm, never override
Explicit `org`/`repo` arguments may **confirm** an existing canonical binding.
They can never establish, complete, or replace one. A request naming a
repository that contradicts the binding fails closed, in both directions:
- a Gitea-Tools-rooted namespace cannot mutate another repository;
- a namespace rooted at another repository cannot mutate Gitea-Tools.
This matters because both-explicit coordinates short-circuit the #530
remote/repo match check, so without this rule a caller could name any repository
and skip validation entirely.
No request-supplied workspace, remote, owner, repository, or worktree can
replace the immutable root.
## Parity is reported per dimension
`gitea_assess_master_parity` reports two separately labelled dimensions:
- `server_implementation` — the Gitea-Tools installation checkout. Its
`startup_head` / `current_head` / `stale` / `restart_required` fields keep
their original meaning, and **only this dimension gates mutations**: the
running process executes the code it started with, so a merged fix is not live
until the daemon restarts.
- `target_repository` — the configured canonical target checkout and its
last-known remote master.
"In parity" is a statement about one dimension, never about the whole system.
Read the dimension you actually care about.
+17
View File
@@ -272,6 +272,15 @@ def load_config(path=None):
)
if not isinstance(data.get("profiles"), dict):
raise ConfigError(f"{path} must be a JSON object with a 'profiles' object")
# #741: the v1 path returns `data` unflattened, so nothing else validates
# the cross-repository binding before gitea_auth.get_profile() reads it.
# Validate it here so every supported loader treats the field identically
# (a relative or blank path must never reach the runtime guard).
for _name, _profile in data["profiles"].items():
if isinstance(_profile, dict):
_validate_canonical_repository_root(
_name, _profile.get("canonical_repository_root")
)
return data
@@ -358,6 +367,14 @@ def _flatten_identity(env_name, svc_name, svc, ident_name, ident):
for key in ("role", "username", "execution_profile", "audit_label"):
if ident.get(key):
profile[key] = ident[key]
# #741: the cross-repository binding must survive flattening. Previously
# this key was silently dropped here, so a v2-environments namespace that
# declared canonical_repository_root fell back to the *installation* root
# and mutated Gitea-Tools instead of its target repository — a fail-open.
# Validate it exactly as the v2-contexts loader does before propagating.
_validate_canonical_repository_root(addr, ident.get("canonical_repository_root"))
if ident.get("canonical_repository_root"):
profile["canonical_repository_root"] = ident["canonical_repository_root"]
return addr, profile
+312 -24
View File
@@ -393,6 +393,35 @@ def _configured_canonical_root() -> tuple[str | None, str | None]:
return crr.configured_canonical_root(profile, os.environ)
def _canonical_local_git_root() -> str:
"""Local git working root for *target-repository* operations (#741).
Two distinct roots exist and must never be conflated:
* ``PROJECT_ROOT`` the Gitea-Tools **installation** checkout, i.e. where
this server script lives. Server implementation/version parity, workflow
and skill file loading, and self-code staleness detection are anchored
here and must stay that way.
* the **canonical target repository root** the working root of the
repository whose issues/PRs/branches the namespace actually mutates.
Every local git invocation that reads or proves *target repository* state
(remote identity, ancestry, worktree inventory, cleanup) must run here, or
a cross-repository namespace silently operates on Gitea-Tools instead.
Returns the resolved canonical target root when one is configured, else
``PROJECT_ROOT`` which preserves the single-repository default exactly.
A configured-but-invalid root is *not* silently replaced by the install
checkout: it is returned as-is so downstream git calls fail rather than
succeed against the wrong repository, and
:func:`_enforce_canonical_repository_root` fails the mutation closed first.
"""
configured, _source = _configured_canonical_root()
if not configured:
return PROJECT_ROOT
return crr.resolve_repo_toplevel(configured) or os.path.realpath(configured)
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
"""Resolve the namespace-scoped workspace root inspected by pre-flight guards.
@@ -1448,6 +1477,12 @@ def _verify_role_mutation_workspace(
git_state = issue_lock_worktree.read_worktree_git_state(
_resolve_preflight_workspace_path(worktree_path)
)
# #741: thread the configured canonical root exactly as
# _resolve_namespace_mutation_context does. Omitting it here made the two
# paths disagree: the #274 branches-only / worktree-membership guards fell
# back to the install checkout and validated Gitea-Tools/branches/ instead
# of the target repository the namespace is actually bound to.
_configured_root, _configured_source = _configured_canonical_root()
assessment = nwb.assess_namespace_mutation_workspace(
role_kind=role,
worktree_path=worktree_path,
@@ -1458,6 +1493,7 @@ def _verify_role_mutation_workspace(
),
profile_name=get_profile().get("profile_name"),
current_branch=git_state.get("current_branch"),
configured_canonical_root=_configured_root,
)
if assessment["block"]:
raise RuntimeError(
@@ -2403,9 +2439,24 @@ def _resolve(remote: str, host: str | None, org: str | None, repo: str | None):
filled_org = False
filled_repo = False
if not org_explicit or not repo_explicit:
parsed = remote_repo_guard.parse_org_repo_from_remote_url(
_local_git_remote_url(remote)
)
parsed = None
# #741: for a cross-repository namespace, prefer the canonical root's
# own repository identity. `_local_git_remote_url` looks a remote up by
# *name*, but a target checkout commonly names its remote `origin`
# rather than `prgs`; that lookup then returns None and the omitted
# coordinates silently fell through to the REMOTES default target —
# i.e. a completely unrelated repository. `_canonical_repository_slug`
# probes the candidate remote names against the canonical root.
try:
canonical_slug, _reasons = _canonical_repository_slug(get_profile(), remote)
except Exception:
canonical_slug = None
if canonical_slug:
parsed = session_ctx.parse_repository_slug(canonical_slug)
if not parsed:
parsed = remote_repo_guard.parse_org_repo_from_remote_url(
_local_git_remote_url(remote)
)
if parsed:
if not org_explicit:
resolved_org = parsed[0]
@@ -2413,6 +2464,40 @@ def _resolve(remote: str, host: str | None, org: str | None, repo: str | None):
if not repo_explicit:
resolved_repo = parsed[1]
filled_repo = True
# #741: explicit caller coordinates may *confirm* a cross-repository
# canonical binding but must never override it. Both-explicit coordinates
# short-circuit the #530 guard (remote_repo_guard.assess_remote_repo_match),
# so without this check a caller could name any repository and skip
# validation entirely. Unconfigured (single-repository) namespaces yield no
# canonical slug and are unaffected. An unresolvable configured root is not
# blocked here — reads must keep working; the fail-closed decision belongs
# to _enforce_canonical_repository_root at the mutation gate.
if org_explicit or repo_explicit:
try:
canonical_slug, _canonical_reasons = _canonical_repository_slug(
get_profile(), remote
)
except Exception:
canonical_slug = None
bound = (
session_ctx.parse_repository_slug(canonical_slug)
if canonical_slug
else None
)
if bound:
override = session_ctx.assess_repository_override(
requested_org=org,
requested_repo=repo,
bound_org=bound[0],
bound_repo=bound[1],
)
if override.get("block"):
raise RuntimeError(
"Canonical repository binding guard (#741): "
+ "; ".join(override.get("reasons") or [])
+ ". Explicit org/repo may confirm the configured canonical "
"repository binding but can never override it."
)
_enforce_remote_repo_guard(
remote,
resolved_org,
@@ -3067,7 +3152,7 @@ def gitea_lock_issue(
return blocked
resolved_worktree = issue_lock_worktree.resolve_author_worktree_path(
worktree_path, PROJECT_ROOT
worktree_path, _canonical_local_git_root()
)
h, o, r = _resolve(remote, host, org, repo)
active_lease_block = issue_lock_store.assess_same_issue_lease_conflict(
@@ -3369,7 +3454,7 @@ def gitea_create_pr(
locked_worktree = lock_data.get("worktree_path")
worktree_check = issue_lock_worktree.verify_pr_worktree_matches_lock(
locked_worktree, worktree_path, PROJECT_ROOT
locked_worktree, worktree_path, _canonical_local_git_root()
)
if worktree_check["block"]:
raise ValueError(worktree_check["reasons"][0])
@@ -8499,13 +8584,24 @@ def gitea_merge_pr(
def _local_git_remote_url(remote_name: str) -> str | None:
"""Best-effort local ``git remote get-url`` for trust-gate corroboration."""
"""Best-effort local ``git remote get-url`` for trust-gate corroboration.
#741: this runs in the **canonical target repository root**, not the
Gitea-Tools installation checkout. Every consumer of this function derives
*target repository identity* from it (``_resolve``, the #530 remote/repo
guard, the anti-stomp org/repo fill, ``_workspace_repository_slug``), so
running it in ``PROJECT_ROOT`` inverted those guards for a cross-repository
namespace: the genuinely bound target was rejected while Gitea-Tools was
accepted. Single-repository namespaces are unaffected
:func:`_canonical_local_git_root` returns ``PROJECT_ROOT`` when no
cross-repository binding is configured.
"""
try:
proc = subprocess.run(
["git", "remote", "get-url", remote_name],
capture_output=True,
text=True,
cwd=PROJECT_ROOT,
cwd=_canonical_local_git_root(),
)
if proc.returncode != 0:
return None
@@ -9075,7 +9171,7 @@ def gitea_cleanup_merged_pr_branch(
else f"origin/{target_branch}"
)
head_on_target = merged_cleanup_reconcile.is_head_ancestor_of_ref(
PROJECT_ROOT,
_canonical_local_git_root(),
head_sha,
target_ref,
)
@@ -9110,7 +9206,7 @@ def gitea_cleanup_merged_pr_branch(
repo=r,
branch=head_branch,
pr_number=pr_number,
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
auth=auth,
base_api=base,
)
@@ -9605,7 +9701,7 @@ def gitea_capture_branches_worktree_snapshot(
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
}
root = PROJECT_ROOT
root = _canonical_local_git_root()
if worktree_path:
root = os.path.realpath(os.path.abspath(worktree_path))
git_root = _get_git_root(root)
@@ -9715,14 +9811,14 @@ def gitea_reconcile_merged_cleanups(
h, o, r, auth, head_ref
)
head_on_master[int(pr["number"])] = merged_cleanup_reconcile.is_head_ancestor_of_ref(
PROJECT_ROOT, head_sha, master_ref
_canonical_local_git_root(), head_sha, master_ref
)
delete_capability_allowed = not _profile_operation_gate("gitea.branch.delete")
# #534: discover reviewer scratch trees and active leases for those PRs.
scratch_candidates = merged_cleanup_reconcile.discover_reviewer_scratch_worktrees(
PROJECT_ROOT
_canonical_local_git_root()
)
active_reviewer_leases: dict[int, bool] = {}
pr_states: dict[int, dict] = {}
@@ -9748,7 +9844,7 @@ def gitea_reconcile_merged_cleanups(
}
report = merged_cleanup_reconcile.build_reconciliation_report(
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
closed_prs=merged_closed,
open_prs=open_prs,
remote_branch_exists=remote_branch_exists,
@@ -9788,7 +9884,7 @@ def gitea_reconcile_merged_cleanups(
repo=r,
branch=head_branch,
pr_number=pr_num_int,
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
auth=auth,
base_api=base,
)
@@ -9872,7 +9968,7 @@ def gitea_reconcile_merged_cleanups(
if local_assessment.get("safe_to_remove_worktree"):
result = merged_cleanup_reconcile.remove_local_worktree(
PROJECT_ROOT,
_canonical_local_git_root(),
head_branch,
worktree_path=local_assessment.get("worktree_path"),
)
@@ -9882,7 +9978,7 @@ def gitea_reconcile_merged_cleanups(
if not scratch.get("safe_to_remove_worktree"):
continue
result = merged_cleanup_reconcile.remove_reviewer_scratch_worktree(
PROJECT_ROOT, scratch.get("worktree_path") or ""
_canonical_local_git_root(), scratch.get("worktree_path") or ""
)
actions.append({"action": "remove_reviewer_scratch_worktree", **result})
@@ -9980,7 +10076,7 @@ def gitea_assess_already_landed_reconciliation(
assessment = reconciliation_workflow.assess_open_pr_reconciliation(
pr=pr,
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
remote=remote,
target_branch=target_branch,
)
@@ -10076,14 +10172,14 @@ def gitea_scan_already_landed_open_prs(
}
target_fetch = reconciliation_workflow.fetch_target_branch(
PROJECT_ROOT, remote, target_branch
_canonical_local_git_root(), remote, target_branch
)
candidates: list[dict] = []
for pr in open_prs:
assessment = reconciliation_workflow.assess_open_pr_reconciliation(
pr=pr,
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
remote=remote,
target_branch=target_branch,
target_fetch=target_fetch,
@@ -10180,7 +10276,7 @@ def gitea_audit_worktree_cleanup(
active_issue_branches.add(str(lock["branch_name"]).strip())
report = worktree_cleanup_audit.audit_branches_directory(
PROJECT_ROOT,
_canonical_local_git_root(),
open_pr_branches=open_pr_branches,
active_issue_branches=active_issue_branches,
now=datetime.now(timezone.utc),
@@ -10263,7 +10359,7 @@ def gitea_reconcile_already_landed_pr(
assessment = already_landed_reconcile.assess_already_landed_reconciliation(
pr=pr,
project_root=PROJECT_ROOT,
project_root=_canonical_local_git_root(),
remote=remote,
target_branch=target_branch,
)
@@ -10468,7 +10564,7 @@ def gitea_reconcile_superseded_by_merged_pr(
)
target_fetch = already_landed_reconcile.fetch_target_branch(
PROJECT_ROOT, remote, target_branch
_canonical_local_git_root(), remote, target_branch
)
superseding_head_sha = (
(superseding_pr.get("head") or {}).get("sha")
@@ -10478,7 +10574,7 @@ def gitea_reconcile_superseded_by_merged_pr(
ancestor = None
if target_fetch.get("success"):
ancestor = already_landed_reconcile.is_head_ancestor_of_ref(
PROJECT_ROOT,
_canonical_local_git_root(),
superseding_head_sha,
target_fetch.get("target_ref") or f"{remote}/{target_branch}",
)
@@ -11622,6 +11718,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,
@@ -11811,6 +11912,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,
@@ -15545,7 +15831,7 @@ def gitea_update_pr_branch_by_merge(
control_clean = True
try:
porcelain = _get_workspace_porcelain(PROJECT_ROOT)
porcelain = _get_workspace_porcelain(_canonical_local_git_root())
# Untracked-only dirt is ignored; tracked edits contaminate control.
tracked_dirty = [
line for line in (porcelain or "").splitlines()
@@ -16550,6 +16836,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],
*,
+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",
+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,768 @@
"""Complete PROJECT_ROOT elimination in cross-repository MCP operations (#741).
#706 introduced the immutable ``canonical_repository_root``; #739/#740 routed
three consumption paths through it. This module covers the paths #740 left
behind, whose shape is uniform: the *filesystem* guards were migrated to the
canonical root, but *repository identity* still bottomed out in
``_local_git_remote_url``'s hardcoded ``cwd=PROJECT_ROOT`` — always the
Gitea-Tools installation checkout.
The consequences asserted here:
* **Identity inversion.** ``_resolve``, the #530 remote/repo guard and the
anti-stomp org/repo fill all derived the *target* repository from the
*install* checkout, so a cross-repository namespace resolved Gitea-Tools
coordinates while its branch/parity facts came from the target repo.
* **Guard disagreement.** ``_verify_role_mutation_workspace`` omitted
``configured_canonical_root``, so the #274 branches-only / worktree-membership
guards validated ``Gitea-Tools/branches/`` rather than the bound target.
* **Explicit-coordinate override.** Both-explicit ``org``/``repo`` short-circuit
``assess_remote_repo_match``, so caller coordinates bypassed validation
entirely rather than merely *confirming* the binding.
* **Configuration fail-open.** ``_flatten_identity`` silently dropped
``canonical_repository_root``, so a v2-``environments`` namespace fell back to
the install root; the v1 path never validated the field at all.
A role-by-operation matrix drives author, reviewer, merger and reconciler
against: correct cross-repository target, wrong repository, wrong worktree,
explicit matching coordinates, explicit mismatched coordinates, missing
canonical root, invalid canonical root, immutable binding after first bind, and
request-override attempts.
Real git repositories are used throughout. No network calls are made, no branch
is deleted, and no merge is performed.
"""
from __future__ import annotations
import json
import os
import subprocess
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import canonical_repository_root as crr # noqa: E402
import gitea_config # noqa: E402
import gitea_mcp_server as srv # noqa: E402
import mcp_server # noqa: E402
import session_context_binding as session_ctx # noqa: E402
INSTALL_ORG = "Scaled-Tech-Consulting"
INSTALL_REPO = "Gitea-Tools"
INSTALL_SLUG = f"{INSTALL_ORG}/{INSTALL_REPO}"
INSTALL_URL = f"https://gitea.prgs.cc/{INSTALL_SLUG}.git"
TARGET_ORG = "Scaled-Tech-Consulting"
TARGET_REPO = "mcp-control-plane"
TARGET_SLUG = f"{TARGET_ORG}/{TARGET_REPO}"
TARGET_URL = f"https://gitea.prgs.cc/{TARGET_SLUG}.git"
THIRD_REPO = "Timesheet"
THIRD_SLUG = f"{INSTALL_ORG}/{THIRD_REPO}"
# tests/conftest.py installs an autouse fixture
# (mutation_profile_fixture.install_deterministic_remote_urls) that *permanently
# reassigns* srv._local_git_remote_url to a stub mapping remote names to fixed
# URLs. That stub deliberately ignores the working directory, which is exactly
# the behaviour this module must verify — so these tests would silently assert
# against the stub rather than production code in a full-suite run. Capture the
# genuine implementation at import time (before any fixture executes) and
# reinstall it per test.
_REAL_LOCAL_GIT_REMOTE_URL = srv._local_git_remote_url
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:
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)
# Distinct content per repo: identical seed content, author and timestamp
# otherwise produce byte-identical commits and therefore an identical SHA,
# which would make the parity-dimension assertions vacuously true.
(path / "README.md").write_text(f"seed {remote_url}\n")
_git(str(path), "add", "README.md")
_git(str(path), "commit", "-q", "-m", f"seed {remote_name}")
return os.path.realpath(str(path))
_ROLE_OPS = {
"author": (
[
"gitea.read",
"gitea.issue.create",
"gitea.issue.comment",
"gitea.pr.create",
"gitea.pr.comment",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
],
["gitea.pr.approve", "gitea.pr.merge", "gitea.pr.request_changes"],
),
"reviewer": (
[
"gitea.read",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.comment",
"gitea.issue.comment",
],
["gitea.pr.create", "gitea.branch.push", "gitea.pr.merge"],
),
"merger": (
["gitea.read", "gitea.pr.merge", "gitea.pr.comment"],
[
"gitea.pr.create",
"gitea.branch.push",
"gitea.pr.approve",
"gitea.pr.request_changes",
],
),
"reconciler": (
["gitea.read", "gitea.pr.close", "gitea.pr.comment", "gitea.branch.delete"],
[
"gitea.pr.create",
"gitea.branch.push",
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.request_changes",
],
),
}
ROLES = tuple(_ROLE_OPS)
def _profile(role: str, *, canonical_root: str | None = None) -> dict:
allowed, forbidden = _ROLE_OPS[role]
profile = {
"enabled": True,
"context": "prgs",
"role": role,
"username": "jcwalker3",
"base_url": "https://gitea.prgs.cc",
"auth": {"type": "env", "name": f"GITEA_TOKEN_PRGS_{role.upper()}"},
"allowed_operations": list(allowed),
"forbidden_operations": list(forbidden),
"execution_profile": f"prgs-{role}",
"allowed_repositories": [TARGET_SLUG],
}
if canonical_root is not None:
profile["canonical_repository_root"] = canonical_root
return profile
def _config(profiles: dict) -> dict:
return {
"version": 2,
"rules": {"allow_runtime_switching": True},
"contexts": {
"prgs": {
"enabled": True,
"gitea": {"enabled": True, "base_url": "https://gitea.prgs.cc"},
}
},
"profiles": profiles,
}
class _CrossRepoHarness(unittest.TestCase):
"""Real install + target git repos, temp profiles.json, no network."""
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self.tmp = self._dir.name
self.config_path = os.path.join(self.tmp, "profiles.json")
# The real install checkout carries a remote literally named `prgs`;
# a freshly cloned target repository normally names its remote `origin`.
# Reproducing that asymmetry is the point: identity derivation must not
# depend on the remote happening to share the `remote=` argument's name.
self.install_root = _init_repo(
Path(self.tmp) / "install", INSTALL_URL, remote_name="prgs"
)
self.target_root = _init_repo(
Path(self.tmp) / "target", TARGET_URL, remote_name="origin"
)
self.third_root = _init_repo(
Path(self.tmp) / "third", f"https://gitea.prgs.cc/{THIRD_SLUG}.git"
)
self.not_a_repo = os.path.join(self.tmp, "plain-dir")
os.makedirs(self.not_a_repo, exist_ok=True)
session_ctx._reset_session_context_for_testing()
gitea_config._active_profile_override = None
mcp_server._IDENTITY_CACHE.clear()
srv._MUTATION_AUTHORITY = None
# PROJECT_ROOT is wherever the server file physically lives; pin it to a
# real Gitea-Tools-identified checkout so "install-derived" is
# deterministic regardless of the developer's layout.
self._project_root = patch.object(srv, "PROJECT_ROOT", self.install_root)
self._project_root.start()
# Undo the autouse deterministic-remote stub for this module only.
self._real_remote_url = patch.object(
srv, "_local_git_remote_url", _REAL_LOCAL_GIT_REMOTE_URL
)
self._real_remote_url.start()
def tearDown(self):
self._real_remote_url.stop()
self._project_root.stop()
session_ctx._reset_session_context_for_testing()
gitea_config._active_profile_override = None
mcp_server._IDENTITY_CACHE.clear()
srv._MUTATION_AUTHORITY = None
self._dir.cleanup()
def _write_config(self, profiles: dict) -> None:
with open(self.config_path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(_config(profiles)))
def _env(self, role: str, **extra) -> dict:
env = {
"GITEA_MCP_CONFIG": self.config_path,
"GITEA_MCP_PROFILE": f"prgs-{role}",
"GITEA_TOKEN_PRGS_AUTHOR": "t",
"GITEA_TOKEN_PRGS_REVIEWER": "t",
"GITEA_TOKEN_PRGS_MERGER": "t",
"GITEA_TOKEN_PRGS_RECONCILER": "t",
}
env.update(extra)
return env
def _bind(self, role: str, canonical_root: str | None):
"""Activate *role* with *canonical_root* and return an env patch ctx."""
self._write_config(
{f"prgs-{role}": _profile(role, canonical_root=canonical_root)}
)
return patch.dict(os.environ, self._env(role), clear=True)
# ===========================================================================
# 1. The central helper (single source of root resolution)
# ===========================================================================
class TestCanonicalLocalGitRoot(_CrossRepoHarness):
"""_canonical_local_git_root is the one place a target root is derived."""
def test_unconfigured_namespace_uses_installation_root(self):
with self._bind("author", None):
self.assertEqual(srv._canonical_local_git_root(), self.install_root)
def test_configured_namespace_uses_target_root(self):
with self._bind("author", self.target_root):
self.assertEqual(srv._canonical_local_git_root(), self.target_root)
def test_configured_root_resolves_to_git_toplevel(self):
nested = os.path.join(self.target_root, "branches", "wt")
os.makedirs(nested, exist_ok=True)
with self._bind("author", nested):
# A subdirectory of the target repo still resolves to its toplevel.
self.assertEqual(srv._canonical_local_git_root(), self.target_root)
def test_invalid_root_never_silently_becomes_installation_root(self):
"""A configured-but-broken root must not fall back to Gitea-Tools."""
with self._bind("author", self.not_a_repo):
resolved = srv._canonical_local_git_root()
self.assertNotEqual(resolved, self.install_root)
self.assertEqual(resolved, os.path.realpath(self.not_a_repo))
def test_env_override_beats_profile_binding(self):
self._write_config(
{"prgs-author": _profile("author", canonical_root=self.third_root)}
)
env = self._env("author", GITEA_CANONICAL_REPOSITORY_ROOT=self.target_root)
with patch.dict(os.environ, env, clear=True):
self.assertEqual(srv._canonical_local_git_root(), self.target_root)
# ===========================================================================
# 2. Repository identity — the upstream inversion
# ===========================================================================
class TestRepositoryIdentityDerivation(_CrossRepoHarness):
"""_local_git_remote_url and its consumers must read the target repo."""
def test_remote_url_reads_target_not_installation(self):
with self._bind("author", self.target_root):
self.assertEqual(srv._local_git_remote_url("origin"), TARGET_URL)
def test_remote_url_unconfigured_still_reads_installation(self):
with self._bind("author", None):
# The install checkout's remote is named `prgs`, matching production.
self.assertEqual(srv._local_git_remote_url("prgs"), INSTALL_URL)
def test_workspace_slug_follows_canonical_root(self):
with self._bind("author", self.target_root):
self.assertEqual(srv._workspace_repository_slug("origin"), TARGET_SLUG)
def test_workspace_slug_unconfigured_is_installation(self):
with self._bind("author", None):
self.assertEqual(srv._workspace_repository_slug("prgs"), INSTALL_SLUG)
def test_every_role_derives_the_same_target_identity(self):
for role in ROLES:
with self.subTest(role=role):
with self._bind(role, self.target_root):
self.assertEqual(
srv._workspace_repository_slug("origin"), TARGET_SLUG
)
# ===========================================================================
# 3. _resolve — omitted and explicit coordinates
# ===========================================================================
class TestResolveTargetCoordinates(_CrossRepoHarness):
"""Omitted coordinates follow the binding; explicit ones may only confirm."""
def test_omitted_coordinates_resolve_to_target_repository(self):
for role in ROLES:
with self.subTest(role=role):
with self._bind(role, self.target_root):
_host, org, repo = srv._resolve("prgs", None, None, None)
self.assertEqual((org, repo), (TARGET_ORG, TARGET_REPO))
def test_omitted_coordinates_unconfigured_resolve_to_installation(self):
with self._bind("author", None):
_host, org, repo = srv._resolve("prgs", None, None, None)
self.assertEqual((org, repo), (INSTALL_ORG, INSTALL_REPO))
def test_explicit_matching_coordinates_confirm_the_binding(self):
for role in ROLES:
with self.subTest(role=role):
with self._bind(role, self.target_root):
_host, org, repo = srv._resolve(
"prgs", None, TARGET_ORG, TARGET_REPO
)
self.assertEqual((org, repo), (TARGET_ORG, TARGET_REPO))
def test_explicit_mismatched_repository_cannot_override_binding(self):
for role in ROLES:
with self.subTest(role=role):
with self._bind(role, self.target_root):
with self.assertRaises(RuntimeError) as ctx:
srv._resolve("prgs", None, TARGET_ORG, INSTALL_REPO)
msg = str(ctx.exception)
self.assertIn("#741", msg)
self.assertIn("fail closed", msg)
def test_explicit_mismatched_organization_cannot_override_binding(self):
with self._bind("author", self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, "SomeoneElse", TARGET_REPO)
def test_gitea_tools_rooted_namespace_cannot_target_control_plane(self):
"""Direction 1: an install-rooted namespace must not reach the target."""
with self._bind("author", self.install_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, TARGET_ORG, TARGET_REPO)
def test_control_plane_rooted_namespace_cannot_target_gitea_tools(self):
"""Direction 2: a target-rooted namespace must not reach Gitea-Tools."""
with self._bind("author", self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, INSTALL_ORG, INSTALL_REPO)
def test_unconfigured_namespace_keeps_existing_explicit_behaviour(self):
"""Same-repository behaviour is unchanged (no new fail-closed path)."""
with self._bind("author", None):
_host, org, repo = srv._resolve("prgs", None, INSTALL_ORG, INSTALL_REPO)
self.assertEqual((org, repo), (INSTALL_ORG, INSTALL_REPO))
# ===========================================================================
# 4. #274 workspace guard — the two guard paths must agree
# ===========================================================================
class TestMutationWorkspaceGuardBinding(_CrossRepoHarness):
"""Both guard paths must agree on which repository they protect."""
def _contexts(self, worktree: str | None = None):
return srv._resolve_namespace_mutation_context(worktree)
def test_mutation_context_uses_target_root(self):
with self._bind("author", self.target_root):
self.assertEqual(self._contexts()["canonical_repo_root"], self.target_root)
def test_mutation_context_unconfigured_uses_installation_root(self):
with self._bind("author", None):
self.assertEqual(self._contexts()["canonical_repo_root"], self.install_root)
def test_role_mutation_workspace_guard_agrees_with_mutation_context(self):
"""The omitted configured_canonical_root made these two disagree."""
import namespace_workspace_binding as nwb
for role in ROLES:
with self.subTest(role=role):
with self._bind(role, self.target_root):
configured, _src = srv._configured_canonical_root()
assessment = nwb.assess_namespace_mutation_workspace(
role_kind=role,
worktree_path=None,
worktree=None,
process_project_root=srv.PROJECT_ROOT,
profile_name=f"prgs-{role}",
configured_canonical_root=configured,
)
self.assertEqual(
assessment["canonical_repo_root"],
self._contexts()["canonical_repo_root"],
)
self.assertEqual(
assessment["canonical_repo_root"], self.target_root
)
def test_wrong_worktree_is_rejected_against_target_root(self):
"""A worktree belonging to the install repo is not in the target repo."""
import author_mutation_worktree as amw
with self._bind("author", self.target_root):
foreign = os.path.join(self.install_root, "branches", "wt")
os.makedirs(foreign, exist_ok=True)
membership = amw.assess_workspace_repo_membership(
workspace_path=foreign,
canonical_repo_root=self.target_root,
)
self.assertTrue(membership["block"])
# ===========================================================================
# 5. Canonical-root validation — missing / invalid / mismatched
# ===========================================================================
class TestCanonicalRootFailClosed(_CrossRepoHarness):
"""Missing, invalid, ambiguous and mismatched roots fail closed."""
def _assess(self, value, *, expected=TARGET_SLUG, require=True):
return crr.assess_canonical_repository_root(
configured_value=value,
source="test",
expected_slug=expected,
process_project_root=self.install_root,
remote="origin",
require_binding=require,
)
def test_missing_root_fails_closed_when_binding_required(self):
result = self._assess(None)
self.assertTrue(result["block"])
self.assertFalse(result["configured"])
def test_missing_root_is_the_single_repo_default_when_not_required(self):
result = self._assess(None, expected=None, require=False)
self.assertFalse(result["block"])
self.assertEqual(result["canonical_repo_root"], self.install_root)
def test_nonexistent_root_fails_closed(self):
result = self._assess(os.path.join(self.tmp, "nope"))
self.assertTrue(result["block"])
self.assertIn("does not exist", " ".join(result["reasons"]))
def test_non_git_directory_fails_closed(self):
result = self._assess(self.not_a_repo)
self.assertTrue(result["block"])
self.assertIn("not a git repository", " ".join(result["reasons"]))
def test_mismatched_repository_identity_fails_closed(self):
result = self._assess(self.third_root)
self.assertTrue(result["block"])
self.assertIn("mismatch", " ".join(result["reasons"]))
def test_matching_repository_identity_is_proven(self):
result = self._assess(self.target_root)
self.assertFalse(result["block"])
self.assertEqual(result["resolved_slug"], TARGET_SLUG)
self.assertEqual(result["canonical_repo_root"], self.target_root)
def test_unresolvable_root_yields_fail_closed_reasons_not_fallback(self):
with self._bind("author", self.not_a_repo):
slug, reasons = srv._canonical_repository_slug(srv.get_profile(), "origin")
self.assertIsNone(slug)
self.assertTrue(reasons)
# ===========================================================================
# 6. Immutability — first bind wins, requests cannot replace the root
# ===========================================================================
class TestCanonicalRootImmutability(_CrossRepoHarness):
"""No request-supplied value can establish or swap the pinned root."""
def test_first_bind_pins_target_root(self):
with self._bind("author", self.target_root):
with patch(
"gitea_mcp_server.api_request",
side_effect=lambda *a, **k: {
"login": "jcwalker3",
"full_name": "T",
"id": 1,
"email": "[email protected]",
},
):
srv.gitea_whoami(remote="prgs")
bound = session_ctx.get_session_context()
self.assertEqual(bound["canonical_repository_root"], self.target_root)
def test_binding_is_first_write_wins(self):
with self._bind("author", self.target_root):
session_ctx.seed_session_context_if_unbound(
profile_name="prgs-author",
remote="prgs",
host="gitea.prgs.cc",
identity="jcwalker3",
org=TARGET_ORG,
repository=TARGET_REPO,
role_kind="author",
source="test",
canonical_repository_root=self.target_root,
)
# A second, contradictory seed must not replace the pin.
session_ctx.seed_session_context_if_unbound(
profile_name="prgs-author",
remote="prgs",
host="gitea.prgs.cc",
identity="jcwalker3",
org=INSTALL_ORG,
repository=INSTALL_REPO,
role_kind="author",
source="test-2",
canonical_repository_root=self.install_root,
)
bound = session_ctx.get_session_context()
self.assertEqual(bound["canonical_repository_root"], self.target_root)
self.assertEqual(bound["repository"], TARGET_REPO)
def test_request_supplied_worktree_cannot_replace_the_root(self):
with self._bind("author", self.target_root):
ctx = srv._resolve_namespace_mutation_context(self.install_root)
# The workspace argument may be demoted/inspected, but the canonical
# repository root stays the configured target.
self.assertEqual(ctx["canonical_repo_root"], self.target_root)
def test_request_supplied_coordinates_cannot_replace_the_root(self):
with self._bind("author", self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, INSTALL_ORG, INSTALL_REPO)
self.assertEqual(srv._canonical_local_git_root(), self.target_root)
# ===========================================================================
# 7. Parity dimensions stay separately labelled (#739 F3 preserved)
# ===========================================================================
class TestParityDimensionSeparation(_CrossRepoHarness):
"""Server-implementation parity stays anchored to the install checkout."""
def test_server_dimension_is_installation_not_target(self):
import master_parity_gate
with self._bind("author", self.target_root):
head = master_parity_gate.read_git_head(srv.PROJECT_ROOT)
self.assertEqual(head, _git(self.install_root, "rev-parse", "HEAD"))
self.assertNotEqual(head, _git(self.target_root, "rev-parse", "HEAD"))
def test_target_dimension_reads_the_target_checkout(self):
with self._bind("author", self.target_root):
self.assertEqual(
_git(srv._canonical_local_git_root(), "rev-parse", "HEAD"),
_git(self.target_root, "rev-parse", "HEAD"),
)
# ===========================================================================
# 8. Configuration loaders validate the binding consistently (AC13)
# ===========================================================================
class TestConfigurationLoaderValidation(unittest.TestCase):
"""Every supported loader treats canonical_repository_root identically."""
def setUp(self):
self._dir = tempfile.TemporaryDirectory()
self.tmp = self._dir.name
def tearDown(self):
self._dir.cleanup()
def _write(self, data: dict) -> str:
path = os.path.join(self.tmp, "profiles.json")
with open(path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(data))
return path
# --- v1 ---------------------------------------------------------------
def _v1(self, root):
return {
"version": 1,
"profiles": {
"prgs-author": {
"base_url": "https://gitea.prgs.cc",
"auth": {"type": "env", "name": "GITEA_TOKEN"},
"canonical_repository_root": root,
}
},
}
def test_v1_rejects_relative_canonical_root(self):
path = self._write(self._v1("relative/path"))
with self.assertRaises(gitea_config.ConfigError) as ctx:
gitea_config.load_config(path)
self.assertIn("absolute", str(ctx.exception))
def test_v1_rejects_blank_canonical_root(self):
path = self._write(self._v1(" "))
with self.assertRaises(gitea_config.ConfigError):
gitea_config.load_config(path)
def test_v1_accepts_absolute_canonical_root(self):
path = self._write(self._v1("/abs/target"))
loaded = gitea_config.load_config(path)
self.assertEqual(
loaded["profiles"]["prgs-author"]["canonical_repository_root"],
"/abs/target",
)
def test_v1_without_the_field_is_unchanged(self):
data = self._v1("/abs/target")
del data["profiles"]["prgs-author"]["canonical_repository_root"]
loaded = gitea_config.load_config(self._write(data))
self.assertNotIn("canonical_repository_root", loaded["profiles"]["prgs-author"])
# --- v2 environments --------------------------------------------------
def _v2_env(self, root):
ident = {
"auth": {"type": "env", "name": "GITEA_TOKEN"},
"base_url": "https://gitea.prgs.cc",
"username": "jcwalker3",
"allowed_operations": ["gitea.read"],
}
if root is not None:
ident["canonical_repository_root"] = root
return {
"version": 2,
"environments": {
"prgs": {"services": {"gitea": {"identities": {"author": ident}}}}
},
}
def test_v2_environments_propagates_canonical_root(self):
"""Regression: the field was silently dropped during flattening."""
loaded = gitea_config.load_config(self._write(self._v2_env("/abs/target")))
profile = loaded["profiles"]["prgs.gitea.author"]
self.assertEqual(profile["canonical_repository_root"], "/abs/target")
def test_v2_environments_rejects_relative_canonical_root(self):
with self.assertRaises(gitea_config.ConfigError) as ctx:
gitea_config.load_config(self._write(self._v2_env("relative/path")))
self.assertIn("absolute", str(ctx.exception))
def test_v2_environments_without_the_field_is_unchanged(self):
loaded = gitea_config.load_config(self._write(self._v2_env(None)))
self.assertNotIn(
"canonical_repository_root", loaded["profiles"]["prgs.gitea.author"]
)
# --- v2 contexts (already validated; guard against regression) ---------
def test_v2_contexts_still_rejects_relative_canonical_root(self):
data = {
"version": 2,
"contexts": {
"prgs": {
"enabled": True,
"gitea": {"enabled": True, "base_url": "https://gitea.prgs.cc"},
}
},
"profiles": {
"prgs-author": {
"enabled": True,
"context": "prgs",
"username": "jcwalker3",
"auth": {"type": "env", "name": "GITEA_TOKEN"},
"allowed_operations": ["gitea.read"],
"canonical_repository_root": "relative/path",
}
},
}
with self.assertRaises(gitea_config.ConfigError):
gitea_config.load_config(self._write(data))
# ===========================================================================
# 9. Role-by-operation matrix
# ===========================================================================
class TestRoleOperationMatrix(_CrossRepoHarness):
"""Each role, each cross-repository condition, one assertion per cell."""
def test_correct_target_resolves_for_every_role(self):
for role in ROLES:
with self.subTest(role=role, case="correct-target"):
with self._bind(role, self.target_root):
_h, org, repo = srv._resolve("prgs", None, None, None)
self.assertEqual((org, repo), (TARGET_ORG, TARGET_REPO))
def test_wrong_repository_is_rejected_for_every_role(self):
for role in ROLES:
with self.subTest(role=role, case="wrong-repo"):
with self._bind(role, self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, INSTALL_ORG, INSTALL_REPO)
def test_explicit_matching_coordinates_pass_for_every_role(self):
for role in ROLES:
with self.subTest(role=role, case="explicit-match"):
with self._bind(role, self.target_root):
_h, org, repo = srv._resolve("prgs", None, TARGET_ORG, TARGET_REPO)
self.assertEqual((org, repo), (TARGET_ORG, TARGET_REPO))
def test_explicit_mismatch_fails_closed_for_every_role(self):
for role in ROLES:
with self.subTest(role=role, case="explicit-mismatch"):
with self._bind(role, self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, INSTALL_ORG, THIRD_REPO)
def test_missing_canonical_root_preserves_single_repo_default(self):
for role in ROLES:
with self.subTest(role=role, case="missing-root"):
with self._bind(role, None):
self.assertEqual(srv._canonical_local_git_root(), self.install_root)
def test_invalid_canonical_root_never_becomes_install_root(self):
for role in ROLES:
with self.subTest(role=role, case="invalid-root"):
with self._bind(role, self.not_a_repo):
self.assertNotEqual(
srv._canonical_local_git_root(), self.install_root
)
def test_wrong_worktree_rejected_for_every_role(self):
import author_mutation_worktree as amw
foreign = os.path.join(self.install_root, "branches", "foreign")
os.makedirs(foreign, exist_ok=True)
for role in ROLES:
with self.subTest(role=role, case="wrong-worktree"):
with self._bind(role, self.target_root):
membership = amw.assess_workspace_repo_membership(
workspace_path=foreign,
canonical_repo_root=srv._canonical_local_git_root(),
)
self.assertTrue(membership["block"])
def test_request_override_attempt_rejected_for_every_role(self):
for role in ROLES:
with self.subTest(role=role, case="request-override"):
with self._bind(role, self.target_root):
with self.assertRaises(RuntimeError):
srv._resolve("prgs", None, "Attacker", "evil-repo")
self.assertEqual(srv._canonical_local_git_root(), self.target_root)
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()