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

Merged
sysadmin merged 1 commits from fix/issue-741-canonical-root-consumers into master 2026-07-18 11:23:35 -05:00
Owner

Closes #741.

Summary

#706 introduced the immutable canonical_repository_root; #739/#740 routed three consumption paths through it. Every path #740 left behind 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.

Inventory and classification

Every production consumer of PROJECT_ROOT, _workspace_repository_slug, _local_git_remote_url, _resolve, _enforce_remote_repo_guard, the anti-stomp org/repo fill, and the local git subprocess helpers was inventoried and classified.

Installation scope — preserved, deliberately unchanged: master_parity_gate startup/current head, workflow + schema + skill file loading, review-workflow boundary state, self-code hashing and stale-runtime detection, role_session_router.assess_infra_stop, mirror_refs.sh lookup, the sys.path bootstrap, and process_project_root= arguments (process identity, with the canonical root passed separately).

Canonical target scope — fixed below.

Ambiguous — explicitly classified rather than left to guess: record_pre_review_command and the shell-spawn health probe are classified installation-scoped (neither derives repository identity nor mutates a repository); gitea_update_pr_branch_by_merge's control_clean porcelain probe is classified target-scoped and fixed.

Defects fixed

  • Central helper. New _canonical_local_git_root() is the single place a target-repository root is resolved: the configured canonical root when declared, else PROJECT_ROOT. A configured-but-invalid root is never silently replaced by the install checkout.
  • _local_git_remote_url now runs in the canonical target root, correcting every downstream identity consumer at once (_resolve, the #530 remote/repo guard, the anti-stomp org/repo fill, _workspace_repository_slug) without duplicating root-resolution logic.
  • _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 — the two paths previously disagreed.
  • _resolve remote-name lookup. Omitted coordinates were derived by looking a remote up by name. A target checkout commonly names its remote origin rather than prgs, so the lookup returned None and coordinates fell through to the remote-wide default target — a foreign repository entirely outside the namespace's scope. It now prefers _canonical_repository_slug, which probes candidate remote names against the canonical root.
  • Explicit-coordinate override. Both-explicit org/repo short-circuit the #530 match check, so caller coordinates bypassed validation entirely. They may now only confirm a canonical binding, never establish or override one, and fail closed in both directions.
  • Reconciler and author paths. Ancestry proofs, fetch_target_branch, worktree inventory/snapshot, cleanup and scratch-tree removal, the author worktree derivation in gitea_lock_issue / gitea_create_pr, and the control_clean porcelain probe now use the canonical target root.
  • Configuration fail-open (AC13). v2-environments _flatten_identity silently dropped canonical_repository_root, so such a namespace fell back to the install root — fail-open, not merely unvalidated. It is now validated and propagated. The v1 path validates it too, so all three loaders behave identically.
  • Documentation (AC16). docs/canonical-repository-root.md explains the two roots, which rule applies to a new consumer, why conflating them inverts the guards, the layered validation, the confirm-never-override rule, and the per-dimension parity reporting. No documentation previously mentioned the canonical root at all.

Preserved invariants

Single-repository Gitea-Tools behaviour is unchanged: with no binding configured the helper returns PROJECT_ROOT and every path behaves exactly as before. Role separation, fail-closed semantics, and branches-only enforcement are untouched. The server_implementation vs target_repository parity dimensions from #740 remain separately labelled, and only the server dimension gates mutations.

Tests

tests/test_issue_741_canonical_root_consumers.py — 51 tests, 52 subtests. 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. Both cross-repository directions and all three config loaders are covered. Real git repositories throughout; no network calls, 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, so without this the assertions would silently pass against the stub in a full-suite run.

Results

  • New module: 51 passed, 52 subtests.
  • Full suite: 3408 passed, 6 skipped, 2 failed.
  • Baseline on clean master c908ed605089: 3357 passed, 6 skipped, 2 failed.

Both failures — test_issue_702_review_findings_f1_f6::test_removed_worktree_recovers_before_probe and test_reconciler_supersession_close::test_tool_posts_comment_and_closes_superseded_pr_issue — reproduce identically on clean master and are pre-existing, not introduced here.

git diff --check is clean. No credential, launcher, live-profile, or generated-file changes. PR #740 and PR #743 artifacts are untouched, and no mcp-control-plane namespace configuration is installed by this work.

Commissioning dependency

This PR does not commission the four mcp-control-plane namespace profiles. Remaining before that is safe: independent review and merge of this PR, then a restart of the Gitea-Tools MCP daemons (the running processes execute their startup commit, so merged fixes are not live until restart), then repository-correct commissioning for author, reviewer, merger and reconciler.

Closes #741. ## Summary #706 introduced the immutable `canonical_repository_root`; #739/#740 routed three consumption paths through it. Every path #740 left behind 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. ## Inventory and classification Every production consumer of `PROJECT_ROOT`, `_workspace_repository_slug`, `_local_git_remote_url`, `_resolve`, `_enforce_remote_repo_guard`, the anti-stomp org/repo fill, and the local git subprocess helpers was inventoried and classified. **Installation scope — preserved, deliberately unchanged:** `master_parity_gate` startup/current head, workflow + schema + skill file loading, review-workflow boundary state, self-code hashing and stale-runtime detection, `role_session_router.assess_infra_stop`, `mirror_refs.sh` lookup, the `sys.path` bootstrap, and `process_project_root=` arguments (process identity, with the canonical root passed separately). **Canonical target scope — fixed below.** **Ambiguous — explicitly classified rather than left to guess:** `record_pre_review_command` and the shell-spawn health probe are classified installation-scoped (neither derives repository identity nor mutates a repository); `gitea_update_pr_branch_by_merge`'s `control_clean` porcelain probe is classified target-scoped and fixed. ## Defects fixed - **Central helper.** New `_canonical_local_git_root()` is the single place a target-repository root is resolved: the configured canonical root when declared, else `PROJECT_ROOT`. A configured-but-invalid root is never silently replaced by the install checkout. - **`_local_git_remote_url`** now runs in the canonical target root, correcting every downstream identity consumer at once (`_resolve`, the #530 remote/repo guard, the anti-stomp org/repo fill, `_workspace_repository_slug`) without duplicating root-resolution logic. - **`_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 — the two paths previously disagreed. - **`_resolve` remote-name lookup.** Omitted coordinates were derived by looking a remote up *by name*. A target checkout commonly names its remote `origin` rather than `prgs`, so the lookup returned `None` and coordinates fell through to the remote-wide default target — a foreign repository entirely outside the namespace's scope. It now prefers `_canonical_repository_slug`, which probes candidate remote names against the canonical root. - **Explicit-coordinate override.** Both-explicit `org`/`repo` short-circuit the #530 match check, so caller coordinates bypassed validation entirely. They may now only *confirm* a canonical binding, never establish or override one, and fail closed in both directions. - **Reconciler and author paths.** Ancestry proofs, `fetch_target_branch`, worktree inventory/snapshot, cleanup and scratch-tree removal, the author worktree derivation in `gitea_lock_issue` / `gitea_create_pr`, and the `control_clean` porcelain probe now use the canonical target root. - **Configuration fail-open (AC13).** v2-`environments` `_flatten_identity` silently *dropped* `canonical_repository_root`, so such a namespace fell back to the install root — fail-open, not merely unvalidated. It is now validated and propagated. The v1 path validates it too, so all three loaders behave identically. - **Documentation (AC16).** `docs/canonical-repository-root.md` explains the two roots, which rule applies to a new consumer, why conflating them inverts the guards, the layered validation, the confirm-never-override rule, and the per-dimension parity reporting. No documentation previously mentioned the canonical root at all. ## Preserved invariants Single-repository Gitea-Tools behaviour is unchanged: with no binding configured the helper returns `PROJECT_ROOT` and every path behaves exactly as before. Role separation, fail-closed semantics, and branches-only enforcement are untouched. The `server_implementation` vs `target_repository` parity dimensions from #740 remain separately labelled, and only the server dimension gates mutations. ## Tests `tests/test_issue_741_canonical_root_consumers.py` — 51 tests, 52 subtests. 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. Both cross-repository directions and all three config loaders are covered. Real git repositories throughout; no network calls, 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, so without this the assertions would silently pass against the stub in a full-suite run. ## Results - New module: **51 passed, 52 subtests**. - Full suite: **3408 passed, 6 skipped, 2 failed**. - Baseline on clean master `c908ed605089`: **3357 passed, 6 skipped, 2 failed**. Both failures — `test_issue_702_review_findings_f1_f6::test_removed_worktree_recovers_before_probe` and `test_reconciler_supersession_close::test_tool_posts_comment_and_closes_superseded_pr_issue` — reproduce identically on clean master and are pre-existing, not introduced here. `git diff --check` is clean. No credential, launcher, live-profile, or generated-file changes. PR #740 and PR #743 artifacts are untouched, and no `mcp-control-plane` namespace configuration is installed by this work. ## Commissioning dependency This PR does not commission the four `mcp-control-plane` namespace profiles. Remaining before that is safe: independent review and merge of this PR, then a restart of the Gitea-Tools MCP daemons (the running processes execute their startup commit, so merged fixes are not live until restart), then repository-correct commissioning for author, reviewer, merger and reconciler.
jcwalker3 added 1 commit 2026-07-18 11:04:19 -05:00
#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
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #744
issue: #741
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 13393-582ad210bab7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255
phase: claimed
candidate_head: 61c3a57df5
target_branch: master
target_branch_sha: c908ed6050
last_activity: 2026-07-18T16:14:10Z
expires_at: 2026-07-18T18:14:10Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #744 issue: #741 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 13393-582ad210bab7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255 phase: claimed candidate_head: 61c3a57df57151a41dff02038acf276f62ad36af target_branch: master target_branch_sha: c908ed6050895d50d99a7f5105976d38a8028ad4 last_activity: 2026-07-18T16:14:10Z expires_at: 2026-07-18T18:14:10Z blocker: none
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #744
issue: #741
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 13393-582ad210bab7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255
phase: validating
candidate_head: 61c3a57df5
target_branch: master
target_branch_sha: c908ed6050
last_activity: 2026-07-18T16:18:10Z
expires_at: 2026-07-18T18:18:10Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #744 issue: #741 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 13393-582ad210bab7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255 phase: validating candidate_head: 61c3a57df57151a41dff02038acf276f62ad36af target_branch: master target_branch_sha: c908ed6050895d50d99a7f5105976d38a8028ad4 last_activity: 2026-07-18T16:18:10Z expires_at: 2026-07-18T18:18:10Z blocker: none
sysadmin approved these changes 2026-07-18 11:19:59 -05:00
sysadmin left a comment
Owner

Canonical PR State

STATE:
approved
WHO_IS_NEXT:
merger
NEXT_ACTION:
Acquire or adopt merger lease, recheck head 61c3a57d, merge only with confirmation MERGE PR 744
NEXT_PROMPT:

Merge Gitea-Tools PR #744 after live recheck of head 61c3a57df57151a41dff02038acf276f62ad36af. Use gitea-merger. Do not re-review unless head moved.

WHAT_HAPPENED:
Independent reviewer session sysadmin/prgs-reviewer completed full validation of PR #744 for issue #741 and approved via native gitea_submit_pr_review.
WHY:
Remaining PROJECT_ROOT coupling inverted cross-repository guards; this PR completes target-root consumption and loader fail-closed behavior.
ISSUE:
#741
HEAD_SHA:
61c3a57df5
REVIEW_STATUS:
APPROVED
MERGE_READY:
true
BLOCKERS:
none
VALIDATION:
new module 51 passed 52 subtests; related batch 359 passed; full suite 3408 passed 6 skipped 2 pre-existing failed; baseline defect proven; git diff --check clean; scope exactly 4 files
LAST_UPDATED_BY:
sysadmin
NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=gitea_submit_pr_review; profile=prgs-reviewer; identity=sysadmin; head=61c3a57df57151a41dff02038acf276f62ad36af; namespace=gitea-reviewer

Review summary

Scope is the four expected files only. Install-scoped PROJECT_ROOT consumers preserved. Target-scoped paths use _canonical_local_git_root(). Explicit coordinates confirm-only. v1/v2 loaders validate/propagate canonical_repository_root. Tests cover four roles and fail closed when stub is incorrectly captured. Residual non-blocking: stale historical comments and collection-order sensitivity of real-function capture (fails closed, not vacuous). Do not merge from this session.

Workflow-load: hash 263d0a6cb8a6; schema a7634e7b8689; boundary clean.

## Canonical PR State STATE: approved WHO_IS_NEXT: merger NEXT_ACTION: Acquire or adopt merger lease, recheck head 61c3a57d, merge only with confirmation MERGE PR 744 NEXT_PROMPT: ```text Merge Gitea-Tools PR #744 after live recheck of head 61c3a57df57151a41dff02038acf276f62ad36af. Use gitea-merger. Do not re-review unless head moved. ``` WHAT_HAPPENED: Independent reviewer session sysadmin/prgs-reviewer completed full validation of PR #744 for issue #741 and approved via native gitea_submit_pr_review. WHY: Remaining PROJECT_ROOT coupling inverted cross-repository guards; this PR completes target-root consumption and loader fail-closed behavior. ISSUE: #741 HEAD_SHA: 61c3a57df57151a41dff02038acf276f62ad36af REVIEW_STATUS: APPROVED MERGE_READY: true BLOCKERS: none VALIDATION: new module 51 passed 52 subtests; related batch 359 passed; full suite 3408 passed 6 skipped 2 pre-existing failed; baseline defect proven; git diff --check clean; scope exactly 4 files LAST_UPDATED_BY: sysadmin NATIVE_REVIEW_PROOF: transport=native_mcp; entrypoint=gitea_submit_pr_review; profile=prgs-reviewer; identity=sysadmin; head=61c3a57df57151a41dff02038acf276f62ad36af; namespace=gitea-reviewer ## Review summary Scope is the four expected files only. Install-scoped PROJECT_ROOT consumers preserved. Target-scoped paths use `_canonical_local_git_root()`. Explicit coordinates confirm-only. v1/v2 loaders validate/propagate canonical_repository_root. Tests cover four roles and fail closed when stub is incorrectly captured. Residual non-blocking: stale historical comments and collection-order sensitivity of real-function capture (fails closed, not vacuous). Do not merge from this session. Workflow-load: hash 263d0a6cb8a6; schema a7634e7b8689; boundary clean.
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #744
issue: #741
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 13393-582ad210bab7
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255
phase: released
candidate_head: 61c3a57df5
target_branch: master
target_branch_sha: c908ed6050
last_activity: 2026-07-18T16:20:28Z
expires_at: 2026-07-18T18:20:28Z
blocker: manual-release

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #744 issue: #741 reviewer_identity: sysadmin profile: prgs-reviewer session_id: 13393-582ad210bab7 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr-744-independent-20260718121255 phase: released candidate_head: 61c3a57df57151a41dff02038acf276f62ad36af target_branch: master target_branch_sha: c908ed6050895d50d99a7f5105976d38a8028ad4 last_activity: 2026-07-18T16:20:28Z expires_at: 2026-07-18T18:20:28Z blocker: manual-release
Owner

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #744
issue: #741
reviewer_identity: sysadmin
profile: prgs-merger
session_id: 33673-1d54887a0415
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/merge-pr-744-61c3a57-20260718T122253
phase: claimed
candidate_head: 61c3a57df5
target_branch: master
target_branch_sha: c908ed6050
last_activity: 2026-07-18T16:23:21Z
expires_at: 2026-07-18T18:23:21Z
blocker: none

<!-- mcp-review-lease:v1 --> repo: Scaled-Tech-Consulting/Gitea-Tools pr: #744 issue: #741 reviewer_identity: sysadmin profile: prgs-merger session_id: 33673-1d54887a0415 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/merge-pr-744-61c3a57-20260718T122253 phase: claimed candidate_head: 61c3a57df57151a41dff02038acf276f62ad36af target_branch: master target_branch_sha: c908ed6050895d50d99a7f5105976d38a8028ad4 last_activity: 2026-07-18T16:23:21Z expires_at: 2026-07-18T18:23:21Z blocker: none
sysadmin merged commit b05075fd25 into master 2026-07-18 11:23:35 -05:00
Owner

Stale #332 review-decision lock cleanup (#594)

Status: APPLIED

Manual deletion of session-state files is not the workflow.
This path only clears a lock when the referenced PR is merged/closed.

## Stale #332 review-decision lock cleanup (#594) Status: **APPLIED** - actor: `sysadmin` - profile: `prgs-merger` - timestamp: `2026-07-18T16:23:36.781469+00:00` - last terminal: `approve` on PR #744 - PR state: `closed` (merged=True) - merge_commit_sha: `b05075fd25ea7f3ac2e86d68f18a1a14c803962e` - prior live_mutations_count: `1` - prior profile_identity: `prgs-reviewer` Manual deletion of session-state files is **not** the workflow. This path only clears a lock when the referenced PR is merged/closed.
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#744