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

fix(preflight): resolve workspace repo for session-bound mutation preflight

Browse Source 
Root-cause follow-up to #733. `_resolve` (the actual API-targeting resolver)
already prefers the workspace-aligned git remote over the `REMOTES` default
(bare `prgs` -> Scaled-Tech-Consulting/Timesheet). The shared #604 anti-stomp
preflight (`_run_anti_stomp_preflight`) did not mirror that: for every mutation
task whose call site omitted org/repo it filled the remote-wide `REMOTES`
default, so the #530 repo guard false-positived `wrong_repo` against a
Gitea-Tools checkout and blocked native issue/PR/review/merge/lock/cleanup
mutations at preflight — even though the resolve stage would have targeted the
workspace repo correctly.

Fix: for omitted coordinates, `_run_anti_stomp_preflight` now prefers the
trusted git-remote-derived workspace identity (marking those filled sides
explicit, exactly like `_resolve`) for session-bound mutation tasks. This fixes
every remaining preflight call site at the shared chokepoint without changing
the global `REMOTES` prgs default (still the best-effort last resort when no
workspace identity can be derived). `delete_branch` keeps its explicit
cross-repository target contract (#733) via `_EXPLICIT_TARGET_MUTATION_TASKS`
and still fails closed on omitted coordinates.

Tests: tests/test_preflight_workspace_repo_forwarding.py (session-bound
resolution across 20 tasks, #530-guard pass, explicit-coords passthrough,
no-workspace REMOTES fallback, delete_branch contract preserved). Full suite
3228 passed / 6 skipped / 240 subtests (1 pre-existing env-leak failure in
test_issue_702_review_findings_f1_f6, fails identically on baseline master).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
sysadmin
2026-07-17 23:43:10 -04:00
co-authored by Claude Opus 4.8
parent 11d1d2e99f
commit d0d789abee
2 changed files with 239 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gitea_mcp_server.py
+57 -11
View File
@@ -767,6 +767,15 @@ def _anti_stomp_in_test_mode() -> bool:
return not bool(os.environ.get("GITEA_TEST_FORCE_ANTI_STOMP"))
# Mutation tasks that carry an explicit cross-repository *target* contract and
# must NOT auto-resolve the workspace repository for omitted coordinates. Only
# ``delete_branch`` (#733) qualifies: a destructive raw-branch deletion names
# its org/repo explicitly and fails closed otherwise. Every other mutation is
# session-bound to the single workspace-aligned repository and resolves it from
# the trusted git remote (mirroring :func:`_resolve`).
_EXPLICIT_TARGET_MUTATION_TASKS = frozenset({"delete_branch"})
def _run_anti_stomp_preflight(
task: str | None,
*,
@@ -832,21 +841,56 @@ def _run_anti_stomp_preflight(
local_remote_url = None
org_explicit = org is not None
repo_explicit = repo is not None
filled_org = False
filled_repo = False
if remote:
try:
local_remote_url = _local_git_remote_url(remote)
except Exception:
local_remote_url = None
if resolved_org is None or resolved_repo is None:
try:
rem = _effective_remote(remote)
if rem in REMOTES:
if resolved_org is None:
resolved_org = REMOTES[rem]["org"]
if resolved_repo is None:
resolved_repo = REMOTES[rem]["repo"]
except Exception:
pass
# Session-bound mutations act on the single workspace-aligned
# repository, so prefer the trusted git-remote-derived identity for
# omitted coordinates — exactly as :func:`_resolve` does — instead
# of the remote-wide ``REMOTES`` default *target*. A bare ``prgs``
# default resolves to ``Timesheet`` and false-positived the #530
# repo guard against a ``Gitea-Tools`` checkout, blocking native
# issue/PR/review/merge/lock/cleanup mutations at preflight while
# the actual resolve stage (``_resolve``) would have targeted the
# workspace repo correctly. Workspace-filled sides are intentional
# alignment and are marked explicit so the guard accepts them.
# ``delete_branch`` keeps its explicit cross-repo target contract
# (#733) and is excluded, so it still fails closed on omitted
# coordinates. ``REMOTES`` stays a best-effort last resort only when
# no workspace identity can be derived (preserves the dadeschools
# default-target behavior and the existing unit suite).
workspace_parts = None
if task not in _EXPLICIT_TARGET_MUTATION_TASKS:
try:
workspace_parts = (
remote_repo_guard.parse_org_repo_from_remote_url(
local_remote_url
)
)
except Exception:
workspace_parts = None
if workspace_parts:
if resolved_org is None:
resolved_org = workspace_parts[0]
filled_org = True
if resolved_repo is None:
resolved_repo = workspace_parts[1]
filled_repo = True
if resolved_org is None or resolved_repo is None:
try:
rem = _effective_remote(remote)
if rem in REMOTES:
if resolved_org is None:
resolved_org = REMOTES[rem]["org"]
if resolved_repo is None:
resolved_repo = REMOTES[rem]["repo"]
except Exception:
pass
parity = _current_master_parity()
startup_head = parity.get("startup_head")
@@ -900,8 +944,10 @@ def _run_anti_stomp_preflight(
resolved_org=resolved_org,
resolved_repo=resolved_repo,
local_remote_url=local_remote_url,
org_explicit=org_explicit,
repo_explicit=repo_explicit,
# Workspace-filled sides are intentional alignment for #530 (same
# contract as :func:`_resolve`), so treat them as explicit.
org_explicit=org_explicit or filled_org,
repo_explicit=repo_explicit or filled_repo,
check_repo=check_repo,
profile_name=profile_name,
profile_role=profile_role,
tests/test_preflight_workspace_repo_forwarding.py
+182
View File
@@ -0,0 +1,182 @@
"""Regression matrix: session-bound mutation preflight resolves the workspace repo.
Root-cause follow-up to #733. ``_resolve`` (the actual API-targeting resolver)
already prefers the workspace-aligned git remote over the ``REMOTES`` default
(bare ``prgs`` → ``Scaled-Tech-Consulting/Timesheet``). The shared #604
anti-stomp preflight (``_run_anti_stomp_preflight``) did **not** mirror that: for
every mutation task whose call site omitted ``org``/``repo`` it filled the
remote-wide ``REMOTES`` default, so the #530 repo guard false-positived
``wrong_repo`` against a ``Gitea-Tools`` checkout and blocked native
issue/PR/review/merge/lock/cleanup mutations.
The fix makes ``_run_anti_stomp_preflight`` prefer the trusted, git-remote-
derived workspace identity for omitted coordinates on **session-bound** mutation
tasks (marking those filled sides explicit, exactly like ``_resolve``), while
``delete_branch`` keeps its explicit cross-repository *target* contract (#733)
and continues to fail closed on omitted coordinates.
"""
import os
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import mcp_server
import remote_repo_guard
# Workspace is bound to Gitea-Tools even though the prgs REMOTES default repo is
# Timesheet (the exact cross-repo scenario).
LOCAL_GITEA_TOOLS_URL = "https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools.git"
RECONCILER = {
"profile_name": "prgs-reconciler",
"role": "reconciler",
"allowed_operations": [
"gitea.read", "gitea.issue.comment", "gitea.pr.comment",
"gitea.pr.close", "gitea.branch.delete",
],
"forbidden_operations": ["gitea.pr.create", "gitea.pr.merge"],
"audit_label": "prgs-reconciler",
}
# Session-bound mutation tasks (every MUTATION_TASK except the explicit-target
# delete_branch) must auto-resolve the workspace repository for omitted coords.
SESSION_BOUND_TASKS = [
"create_issue",
"comment_issue",
"close_issue",
"mark_issue",
"lock_issue",
"set_issue_labels",
"create_label",
"create_pr",
"close_pr",
"edit_pr",
"commit_files",
"cleanup_merged_pr_branch",
"cleanup_stale_claims",
"reconcile_merged_cleanups",
"reconcile_already_landed_pr",
"reconcile_close_superseded_pr",
"post_heartbeat",
"review_pr",
"submit_pr_review",
"merge_pr",
]
class _AntiStompResolutionBase(unittest.TestCase):
def setUp(self):
self._remotes = patch.dict(mcp_server.REMOTES, {
"prgs": {
"host": "gitea.prgs.cc",
"org": "Scaled-Tech-Consulting",
"repo": "Timesheet",
},
})
self._remotes.start()
def tearDown(self):
patch.stopall()
def _capture_resolution(self, task, org, repo, *, local_url=LOCAL_GITEA_TOOLS_URL):
"""Drive the live anti-stomp runner and capture the org/repo it resolved
and passed to the pure assessor."""
passthrough = {
"allowed": True, "block": False, "blockers": [], "reasons": [],
"exact_next_action": "proceed", "blocker_kind": None, "checks": {},
}
with patch.dict(
os.environ, {"GITEA_TEST_FORCE_ANTI_STOMP": "1"}, clear=False
), patch(
"mcp_server._local_git_remote_url", return_value=local_url
), patch(
"mcp_server.get_profile", return_value=RECONCILER
), patch.object(
mcp_server.anti_stomp_preflight,
"assess_anti_stomp_preflight",
return_value=passthrough,
) as m:
mcp_server._run_anti_stomp_preflight(
task, remote="prgs", org=org, repo=repo
)
self.assertTrue(m.called)
return m.call_args.kwargs
class TestSessionBoundResolution(_AntiStompResolutionBase):
def test_omitted_coords_resolve_workspace_repo_and_mark_explicit(self):
for task in SESSION_BOUND_TASKS:
with self.subTest(task=task):
kw = self._capture_resolution(task, None, None)
self.assertEqual(
kw["resolved_org"], "Scaled-Tech-Consulting", task
)
self.assertEqual(kw["resolved_repo"], "Gitea-Tools", task)
# Workspace-filled sides are intentional alignment → explicit.
self.assertTrue(kw["org_explicit"], task)
self.assertTrue(kw["repo_explicit"], task)
def test_resolved_workspace_repo_passes_the_530_guard(self):
kw = self._capture_resolution("create_pr", None, None)
assessment = remote_repo_guard.assess_remote_repo_match(
remote="prgs",
resolved_org=kw["resolved_org"],
resolved_repo=kw["resolved_repo"],
local_remote_url=LOCAL_GITEA_TOOLS_URL,
org_explicit=kw["org_explicit"],
repo_explicit=kw["repo_explicit"],
)
self.assertFalse(assessment["block"], assessment)
def test_explicit_caller_coords_are_preserved(self):
kw = self._capture_resolution(
"create_pr", "Scaled-Tech-Consulting", "Gitea-Tools"
)
self.assertEqual(kw["resolved_org"], "Scaled-Tech-Consulting")
self.assertEqual(kw["resolved_repo"], "Gitea-Tools")
self.assertTrue(kw["org_explicit"])
self.assertTrue(kw["repo_explicit"])
def test_no_workspace_identity_falls_back_to_remotes_default(self):
# No derivable local remote → best-effort REMOTES default preserved,
# not marked explicit (so the guard still corroborates when able).
kw = self._capture_resolution("create_pr", None, None, local_url=None)
self.assertEqual(kw["resolved_repo"], "Timesheet")
self.assertFalse(kw["org_explicit"])
self.assertFalse(kw["repo_explicit"])
class TestDeleteBranchContractPreserved(_AntiStompResolutionBase):
"""delete_branch keeps its explicit-target contract (#733): omitted coords
still resolve the remote-wide default and are NOT marked explicit, so the
guard fails closed against a mismatched workspace."""
def test_omitted_coords_still_remote_default_not_explicit(self):
kw = self._capture_resolution("delete_branch", None, None)
self.assertEqual(kw["resolved_repo"], "Timesheet")
self.assertFalse(kw["org_explicit"])
self.assertFalse(kw["repo_explicit"])
assessment = remote_repo_guard.assess_remote_repo_match(
remote="prgs",
resolved_org=kw["resolved_org"],
resolved_repo=kw["resolved_repo"],
local_remote_url=LOCAL_GITEA_TOOLS_URL,
org_explicit=kw["org_explicit"],
repo_explicit=kw["repo_explicit"],
)
self.assertTrue(assessment["block"])
def test_explicit_coords_still_forwarded(self):
kw = self._capture_resolution(
"delete_branch", "Scaled-Tech-Consulting", "Gitea-Tools"
)
self.assertEqual(kw["resolved_repo"], "Gitea-Tools")
self.assertTrue(kw["org_explicit"])
self.assertTrue(kw["repo_explicit"])
if __name__ == "__main__":
unittest.main()