Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 73 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(mcp): consume canonical target root in cross-repository operations (Closes #741)

Browse Source 
#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
This commit is contained in:
sysadmin
2026-07-18 12:02:31 -04:00
co-authored by Claude Opus 4.8
parent c908ed6050
commit 61c3a57df5
4 changed files with 1058 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gitea_mcp_server.py
+120 -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}",
)
@@ -15735,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()