Compare commits

...
Author SHA1 Message Date
sysadminandClaude Opus 4.8 300a4ca08f chore(#681): preserve dirty control-checkout WIP from PR #680 review session
Moves the three uncommitted test-isolation changes off the root/control
checkout into a dedicated issue-681 branch, per issue #681 recovery path
(AC2: retained test-isolation changes land on a dedicated issue branch
with review; never smuggled into #604/PR #680).

Preserved as-is (NOT endorsed) for review:
- gitea_mcp_server.py: test-mode early return in _verify_role_mutation_workspace
- issue_lock_worktree.py: strips *.py lines from porcelain under pytest/unittest
- tests/test_preflight_read_survival.py: patches out branches-only + root-checkout guards

These weaken workspace/dirtiness guards under test and require review before
any adoption; committed here only to restore the control checkout to clean
master without discarding unique work.

Refs #681

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-12 10:35:28 -04:00
sysadmin 22698c1b5f Merge pull request 'feat(guard): block direct stable-branch pushes from MCP workflow sessions (Closes #671)' (#677) from fix/issue-671-block-stable-branch-push into master 2026-07-11 20:05:45 -05:00
sysadmin de27053f74 Merge pull request 'fix: residual preflight/worktree/test-isolation remediations after #673' (#676) from fix/issue-675-residual-preflight-remediation into master 2026-07-11 19:15:56 -05:00
sysadminandClaude Opus 4.8 5933d87647 feat(guard): block direct stable-branch pushes from MCP workflow sessions (Closes #671)
Prevention hardening for the #670 incident (bare direct-to-master commit
2fa97c26 and the PR #654 merger stable-branch push attempt). Worker sessions
must never publish stable branches directly; stable updates land only through
sanctioned Gitea merge tooling or an authorized reconciler path.

New pure module stable_branch_push_guard.py:
- classify_push_command: detects git push <remote> <stable-ref> equivalents
  including refspecs (HEAD:<ref>, +refs/heads/x:refs/heads/<ref>), --force,
  --delete/:<ref>, and --dry-run/-n no-op probes (dry-run still proves intent).
  Fetch/pull and feature-branch pushes are never flagged; sanctioned
  gitea_merge_pr / API merge is not a push.
- assess_root_checkout_local_commit: detects control-checkout commits not on an
  issue feature branch (branches/ worktrees exempt).
- redact_command: strips URL userinfo/token assignments before logging.
- build_contamination_record + assess_contamination_gate: durable marker shape
  and fail-closed gate (reconciler-exempt; comment/lock stay allowed for handoff).

Server wiring (gitea_mcp_server.py, mcp_session_state.py):
- KIND_STABLE_BRANCH_CONTAMINATION durable session marker (per profile identity).
- _enforce_stable_branch_contamination_gate wired into verify_preflight_purity
  so review/merge/close/completion mutations fail closed while contaminated.
- gitea_record_stable_branch_push_attempt: classify + mark on detection.
- gitea_audit_stable_branch_contamination: reconciler-only inspect/clear; a
  worker session can never self-clear.

Tests (AC5): no-op dry-run push, real direct push, sanctioned Gitea merge,
fetch-only, root-checkout local commit, feature-branch push allowed, gate
block/reconciler-exempt, redaction. 51 new tests; full suite 2596 passed.

Docs: llm-project-workflow SKILL.md — universal rule, blocker class, and a
dedicated "Stable Branch Push Protection" section (worker sessions must never
push stable branches directly).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-11 20:07:40 -04:00
sysadmin 72b18143f8 fix(preflight): unify review worktree regex, resolve Web UI NameError, and fix test isolation 2026-07-11 19:49:16 -04:00
sysadmin 8886ce201f fix(preflight): bypass workspace checks in test mode and mock subprocess in alignment tests 2026-07-11 19:49:16 -04:00
sysadmin bee24e2b10 Merge pull request 'fix(test): remediate repository-wide regressions (Closes #673)' (#674) from fix/issue-673-remediate-regressions into master 2026-07-11 18:16:05 -05:00
sysadmin a7aac0df1d fix(test): remediate repository-wide regressions (Closes #673) 2026-07-10 18:09:19 -04:00
sysadmin ec903b0d61 Merge pull request 'feat: lifecycle role/hazard labels and canonical state mapping (#603)' (#654) from feat/issue-603-lifecycle-labels into master 2026-07-10 15:43:47 -05:00
sysadminandClaude Opus 4.8 f34319852e feat: lifecycle role/hazard labels and canonical state mapping (#603)
Add orthogonal role:* (single-active owner) and hazard:* (additive warning)
lifecycle labels plus status:changes-requested, folding the #603 state:*
vocabulary into the canonical status:* set rather than a conflicting parallel
prefix.

- issue_workflow_labels.py: ROLE_/HAZARD_ specs + frozensets; role/hazard
  transition maps and helpers (transition_role_labels, add/clear_hazard_label,
  canonical_role_label, canonical_hazard_label, is_discussion,
  is_implementation_candidate, requires_blocking_reason); state:* -> status:*
  synonym transitions; assess_issue_labels surfaces role/hazard dims and flags
  multiple active role labels
- docs/label-taxonomy.md: role, hazard, state->canonical migration, and
  allocator cross-check sections
- tests: role/hazard/discussion/blocking-reason/state-synonym coverage
- manage_labels.py seeds the new labels automatically via CANONICAL_LABEL_SPECS

Labels remain advisory; control-plane leases (#601) and live PR state stay the
source of truth for mutations (#603 AC2).

Closes #603

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-10 16:17:31 -04:00
sysadmin 2fa97c26fb fix(mcp): load dotenv relative to project root 2026-07-10 15:22:18 -04:00
sysadmin 5ab5fe8583 Merge pull request 'fix: paginate repository-label inventory for gitea_set_issue_labels (Closes #627)' (#629) from fix/issue-627-set-issue-labels-pagination into master
Closes #627

Merges paginated repository-label inventory for gitea_set_issue_labels so later-page labels are not falsely rejected during full-set replacement.
2026-07-10 12:54:23 -05:00
sysadmin f32aaaa3b5 fix: paginate repository-label inventory for gitea_set_issue_labels (#627)
Replace single-page GET labels?limit=100 with api_get_all-backed
_repo_label_id_map so later-page labels (e.g. type:feature,
workflow-hardening) are not falsely rejected during full-set replacement.

Also add post-mutation verification, fix related MCP/CLI label inventory
call sites, and add multi-page regression tests for the #601 reconciler
failure mode.

Closes #627
2026-07-10 13:05:27 -04:00
sysadmin 5347b313ea Merge pull request 'feat: first-class control-plane lease lifecycle (Closes #601)' (#625) from feat/issue-601-first-class-leases into master 2026-07-10 11:23:29 -05:00
33 changed files with 2286 additions and 150 deletions
+67
View File
@@ -39,6 +39,7 @@ one.
| `status:blocked` | Issue is blocked | | `status:blocked` | Issue is blocked |
| `status:needs-review` | Issue work needs review | | `status:needs-review` | Issue work needs review |
| `status:pr-open` | A linked PR is open | | `status:pr-open` | A linked PR is open |
| `status:changes-requested` | Reviewer requested changes on the linked PR |
| `status:approved` | Linked PR is approved | | `status:approved` | Linked PR is approved |
| `status:merged` | Linked PR is merged | | `status:merged` | Linked PR is merged |
| `status:reconcile` | Issue needs reconciliation | | `status:reconcile` | Issue needs reconciliation |
@@ -46,6 +47,72 @@ one.
| `status:duplicate` | Issue is a duplicate | | `status:duplicate` | Issue is a duplicate |
| `status:wontfix` | Issue will not be fixed | | `status:wontfix` | Issue will not be fixed |
## Role Ownership Labels (#603)
A single `role:*` label shows which workflow role currently owns the item. It is
advisory visibility only — the control-plane lease (#601) is the source of truth
for mutation authority. Only one `role:*` label is active at a time; tooling
replaces it on handoff via `transition_role_labels`.
| Label | Use |
| --- | --- |
| `role:author` | Author currently owns the item |
| `role:reviewer` | Reviewer currently owns the item |
| `role:merger` | Merger currently owns the item |
## Hazard Labels (#603)
Hazard labels are orthogonal warning flags. Unlike `status:*` and `role:*`,
**more than one hazard may be active at once**, and a hazard never substitutes
for a live lease / PR-state check. Add/remove with `add_hazard_label` /
`clear_hazard_label`.
| Label | Use |
| --- | --- |
| `hazard:stale-lease` | A stale or expired lease references this item |
| `hazard:workflow-contaminated` | Session/workflow state is contaminated; do not mutate |
| `hazard:conflicted` | Linked PR has merge conflicts |
| `hazard:root-mutation` | Work was mutated in the project root checkout |
| `hazard:manual-state` | Session or lease state was edited manually |
| `hazard:terminal-blocker` | A terminal review/merge lock blocks progress (#332/#602) |
Any item that carries `status:blocked` or any `hazard:*` flag must also have a
blocking-reason / next-action comment (`requires_blocking_reason`).
## `state:*` → canonical mapping (#603 migration)
Issue #603 proposed a parallel `state:*` vocabulary. To avoid a conflicting
second lifecycle prefix, those requested states are folded into the existing
canonical labels rather than introduced as `state:*`. `state:*` is **not** a
supported prefix; use the canonical label on the right.
| Requested `state:*` | Canonical label |
| --- | --- |
| `state:needs-triage` | `status:triage` |
| `state:claimed` | `status:claimed` |
| `state:authoring` | `status:in-progress` |
| `state:needs-review` | `status:needs-review` |
| `state:reviewing` | `status:needs-review` |
| `state:changes-requested` | `status:changes-requested` |
| `state:approved` | `status:approved` |
| `state:merge-ready` | `status:approved` |
| `state:merged` | `status:merged` |
| `state:blocked` | `status:blocked` |
| `state:terminal-blocker` | `hazard:terminal-blocker` |
| `state:abandoned` | `status:wontfix` |
The transition helpers accept these names as synonyms (e.g.
`canonical_status_label("authoring")``status:in-progress`), so callers may use
the #603 wording while a single canonical status stays active.
## Allocator Cross-Check (#603)
Labels are advisory queue hints. The work allocator (#600/#613) uses labels as
one signal but **cross-checks live leases and PR state** and never trusts labels
alone. Discussion issues (`type:discussion`) are excluded from implementation
queues (`is_implementation_candidate`) unless a controller explicitly selects
them.
## Transition Rules ## Transition Rules
Suggested lifecycle: Suggested lifecycle:
+3 -2
View File
@@ -20,14 +20,15 @@ from dotenv import dotenv_values, load_dotenv
import gitea_config import gitea_config
PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))
# Load standard .env if present # Load standard .env if present
load_dotenv() load_dotenv(os.path.join(PROJECT_ROOT, ".env"))
# Dictionary to store configurations parsed dynamically from .env.* files # Dictionary to store configurations parsed dynamically from .env.* files
DYNAMIC_CONFIGS = {} DYNAMIC_CONFIGS = {}
# Scan all files starting with .env in the project root to load multiple configurations # Scan all files starting with .env in the project root to load multiple configurations
PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))
for env_path in glob.glob(os.path.join(PROJECT_ROOT, ".env*")): for env_path in glob.glob(os.path.join(PROJECT_ROOT, ".env*")):
# Skip directories and the example template # Skip directories and the example template
if os.path.basename(env_path) == ".env.example": if os.path.basename(env_path) == ".env.example":
+304 -41
View File
@@ -653,6 +653,10 @@ def verify_preflight_purity(
f"'{task}' (fail closed)" f"'{task}' (fail closed)"
) )
# #671: block review/merge/close/completion mutations while the session is
# contaminated by a direct stable-branch push attempt (reconciler-exempt).
_enforce_stable_branch_contamination_gate(task, remote)
ctx = _resolve_namespace_mutation_context(worktree_path) ctx = _resolve_namespace_mutation_context(worktree_path)
workspace = ctx["workspace_path"] workspace = ctx["workspace_path"]
canonical_root = ctx["canonical_repo_root"] canonical_root = ctx["canonical_repo_root"]
@@ -724,6 +728,9 @@ def _verify_role_mutation_workspace(
task: str | None = None, task: str | None = None,
) -> str: ) -> str:
"""Bind reviewer/merger mutations to the active namespace workspace (#510).""" """Bind reviewer/merger mutations to the active namespace workspace (#510)."""
if _preflight_in_test_mode():
return _resolve_preflight_workspace_path(worktree_path)
# Check running runtimes to prevent stale mutations # Check running runtimes to prevent stale mutations
try: try:
if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ: if "PYTEST_CURRENT_TEST" not in os.environ or "GITEA_FORCE_MCP_RUNTIME_CHECK" in os.environ:
@@ -809,6 +816,80 @@ def _enforce_root_checkout_guard(worktree_path: str | None = None) -> None:
raise RuntimeError(root_checkout_guard.format_root_checkout_guard_error(assessment)) raise RuntimeError(root_checkout_guard.format_root_checkout_guard_error(assessment))
# ── stable-branch push contamination (#671) ──────────────────────────────────
# A worker session that attempts a direct stable-branch push (or a
# root-checkout local commit) is workflow-contaminated. The marker is durable
# (survives daemon process pools like the other session proofs) and keyed per
# profile identity. It fails closed on gated mutations until a reconciler
# audits and clears it.
def _stable_contamination_profile_identity() -> str:
return mcp_session_state.current_profile_identity(
profile_name=get_profile().get("profile_name"),
)
def _load_stable_contamination_marker(remote: str | None = None) -> dict | None:
return mcp_session_state.load_state(
kind=mcp_session_state.KIND_STABLE_BRANCH_CONTAMINATION,
remote=remote,
profile_identity=_stable_contamination_profile_identity(),
)
def _save_stable_contamination_marker(
record: dict,
*,
remote: str | None = None,
) -> dict | None:
return mcp_session_state.save_state(
kind=mcp_session_state.KIND_STABLE_BRANCH_CONTAMINATION,
payload=record,
remote=remote,
profile_identity=_stable_contamination_profile_identity(),
)
def _clear_stable_contamination_marker(
*,
remote: str | None = None,
profile_identity: str | None = None,
) -> None:
mcp_session_state.clear_state(
kind=mcp_session_state.KIND_STABLE_BRANCH_CONTAMINATION,
remote=remote,
profile_identity=profile_identity or _stable_contamination_profile_identity(),
)
def _enforce_stable_branch_contamination_gate(
task: str | None,
remote: str | None = None,
) -> None:
"""#671 AC4: fail closed on gated mutations while contaminated.
Reconciler role is exempt (the sanctioned audit/clear path). Non-gated
tasks (comment_issue, lock_issue) stay allowed so a contaminated worker can
still post the durable audit comment and hand off.
"""
if _preflight_in_test_mode() and not os.environ.get(
"GITEA_TEST_FORCE_STABLE_CONTAMINATION"
):
return
marker = _load_stable_contamination_marker(remote)
if not marker:
return
gate = stable_branch_push_guard.assess_contamination_gate(
marker,
task=task,
actual_role=_actual_profile_role(),
)
if gate["block"]:
raise RuntimeError(
stable_branch_push_guard.format_contamination_gate_error(gate)
)
from mcp.server.fastmcp import FastMCP # noqa: E402 from mcp.server.fastmcp import FastMCP # noqa: E402
from gitea_auth import ( # noqa: E402 from gitea_auth import ( # noqa: E402
@@ -849,6 +930,7 @@ import merge_approval_gate # noqa: E402
import already_landed_reconcile # noqa: E402 import already_landed_reconcile # noqa: E402
import author_mutation_worktree # noqa: E402 import author_mutation_worktree # noqa: E402
import root_checkout_guard # noqa: E402 import root_checkout_guard # noqa: E402
import stable_branch_push_guard # noqa: E402
import remote_repo_guard # noqa: E402 import remote_repo_guard # noqa: E402
import issue_claim_heartbeat # noqa: E402 import issue_claim_heartbeat # noqa: E402
import issue_work_duplicate_gate # noqa: E402 import issue_work_duplicate_gate # noqa: E402
@@ -1216,12 +1298,32 @@ def extract_linked_issue_numbers(text: str | None, branch_name: str | None = Non
return sorted(list(issues)) return sorted(list(issues))
def _repo_label_id_map(base: str, auth: str) -> dict[str, int]: def _repo_label_id_map(base: str, auth: str) -> dict[str, int]:
labels = api_get_all(f"{base}/labels", auth) or [] """Map repository label names to IDs across **all** label pages (#627).
return {
str(lb["name"]): int(lb["id"]) Uses :func:`api_get_all` so inventories larger than Gitea's per-page cap
for lb in labels (50) are complete. Duplicate names keep the **first-seen** id for
if isinstance(lb, dict) and lb.get("name") and lb.get("id") is not None deterministic resolution (fail-open for attach; names still resolve).
} """
labels = api_get_all(f"{base}/labels", auth)
if labels is None:
labels = []
if not isinstance(labels, list):
raise RuntimeError(
"failed to list repository labels: expected a list page sequence, "
f"got {type(labels).__name__}"
)
name_to_id: dict[str, int] = {}
for lb in labels:
if not isinstance(lb, dict):
continue
name = lb.get("name")
lid = lb.get("id")
if not name or lid is None:
continue
key = str(name)
if key not in name_to_id:
name_to_id[key] = int(lid)
return name_to_id
def _issue_label_names(base: str, auth: str, issue_number: int) -> list[str]: def _issue_label_names(base: str, auth: str, issue_number: int) -> list[str]:
issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {} issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {}
@@ -1235,20 +1337,46 @@ def _put_issue_label_names(
names: list[str], names: list[str],
label_ids_by_name: dict[str, int] | None = None, label_ids_by_name: dict[str, int] | None = None,
) -> list[dict]: ) -> list[dict]:
"""Full-set label replacement with complete inventory + post-mutation check.
Missing requested names fail closed before PUT. After PUT, the returned
label set must match the requested names (order-independent) so callers
never silently drop labels (#627).
"""
by_name = label_ids_by_name or _repo_label_id_map(base, auth) by_name = label_ids_by_name or _repo_label_id_map(base, auth)
missing = [name for name in names if name not in by_name] missing = [name for name in names if name not in by_name]
if missing: if missing:
raise RuntimeError( raise RuntimeError(
f"The following labels do not exist on the repository: {missing}. " f"The following labels do not exist on the repository: {missing}. "
"Create the canonical workflow labels first." "Please create them first using gitea_create_label."
) )
ids = [by_name[name] for name in names] ids = [by_name[name] for name in names]
return api_request( res = api_request(
"PUT", "PUT",
f"{base}/issues/{issue_number}/labels", f"{base}/issues/{issue_number}/labels",
auth, auth,
{"labels": ids}, {"labels": ids},
) )
if not isinstance(res, list):
raise RuntimeError(
"Post-mutation label verification failed: expected a list of labels "
f"from Gitea, got {type(res).__name__}."
)
final_names = {
str(lb.get("name"))
for lb in res
if isinstance(lb, dict) and lb.get("name")
}
expected = {str(n) for n in names}
if final_names != expected:
missing_after = sorted(expected - final_names)
extra_after = sorted(final_names - expected)
raise RuntimeError(
"Post-mutation label verification failed: "
f"missing={missing_after} unexpected={extra_after}. "
"Full-set replacement did not match the requested label set."
)
return res
def _transition_issue_status( def _transition_issue_status(
*, *,
@@ -1326,12 +1454,9 @@ def release_in_progress_label(issue_numbers: list[int], remote: str, host: str |
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
try: try:
labels = api_request("GET", f"{base}/labels?limit=100", auth) # Paginated inventory (#627): status labels must resolve even when
label_id = None # the repo has more labels than one Gitea page.
for lb in labels: label_id = _repo_label_id_map(base, auth).get("status:in-progress")
if lb["name"] == "status:in-progress":
label_id = lb["id"]
break
except Exception as exc: except Exception as exc:
return {num: f"error fetching repo labels: {_redact(str(exc))}" for num in issue_numbers} return {num: f"error fetching repo labels: {_redact(str(exc))}" for num in issue_numbers}
@@ -9230,6 +9355,160 @@ def gitea_diagnose_terminal(
} }
@mcp.tool()
def gitea_record_stable_branch_push_attempt(
command: str | None = None,
remote: str = "dadeschools",
ref: str | None = None,
session_id: str | None = None,
mark: bool = True,
current_branch: str | None = None,
head_sha: str | None = None,
remote_master_sha: str | None = None,
is_under_branches: bool | None = None,
ahead_count: int | None = None,
) -> dict:
"""Classify a proposed command for direct stable-branch push intent (#671).
Worker sessions must never publish stable branches (``master``/``main``/
``dev``/...) directly. This tool detects ``git push <remote> master``
equivalents (refspecs, ``HEAD:master``, ``--force``, ``--dry-run`` no-op
intent, ``:master`` delete) and separately a root/control-checkout
local commit not carried by an issue feature branch.
When ``mark`` is true and contamination is detected, a durable
``stable_branch_contamination`` marker is written for the active profile
identity. Subsequent review/merge/close/completion mutations then fail
closed (via the pre-flight gate) until a reconciler audits and clears it.
The stored command summary is redacted; secrets never persist.
Read-only when nothing is detected (or ``mark`` is false). Returns the
classification, any root-checkout assessment, and the marker state.
"""
classification = stable_branch_push_guard.classify_push_command(command)
root_checkout = None
if any(v is not None for v in (current_branch, head_sha, remote_master_sha,
is_under_branches, ahead_count)):
root_checkout = stable_branch_push_guard.assess_root_checkout_local_commit(
current_branch=current_branch,
head_sha=head_sha,
remote_master_sha=remote_master_sha,
is_under_branches=bool(is_under_branches),
ahead_count=ahead_count,
)
push_contam = bool(classification.get("contamination"))
root_contam = bool(root_checkout and root_checkout.get("contamination"))
contaminated = push_contam or root_contam
marker = None
marked = False
if contaminated and mark:
if push_contam:
reason_class = "stable_branch_push"
command_redacted = classification.get("redacted_command")
detail = "; ".join(classification.get("reasons") or [])
resolved_ref = ref or (
(classification.get("stable_refs") or [None])[0]
)
else:
reason_class = "root_checkout_commit"
command_redacted = None
detail = "; ".join(root_checkout.get("reasons") or [])
resolved_ref = ref or root_checkout.get("current_branch")
record = stable_branch_push_guard.build_contamination_record(
reason_class=reason_class,
command_redacted=command_redacted,
session_id=session_id,
remote=remote,
ref=resolved_ref,
role=_actual_profile_role(),
detail=detail,
)
marker = _save_stable_contamination_marker(record, remote=remote)
marked = marker is not None
return {
"classification": classification,
"root_checkout": root_checkout,
"contaminated": contaminated,
"marked": marked,
"marker": marker,
"profile_identity": _stable_contamination_profile_identity(),
"remediation": stable_branch_push_guard.REMEDIATION if contaminated else None,
}
@mcp.tool()
def gitea_audit_stable_branch_contamination(
action: str = "inspect",
remote: str = "dadeschools",
profile_identity: str | None = None,
) -> dict:
"""Reconciler audit of a stable-branch contamination marker (#671).
``action='inspect'`` (default, read-only) loads the durable marker for the
given ``profile_identity`` (or the active session's) and reports it.
``action='clear'`` removes the marker so the contaminated session may
resume gated mutations. Clearing is the reconciler audit path and requires
an active reconciler profile a worker session must never self-clear
(#671 security requirement). ``profile_identity`` targets the contaminated
worker's marker (a reconciler runs under its own identity).
"""
act = (action or "inspect").strip().lower()
target_identity = (profile_identity or "").strip() or _stable_contamination_profile_identity()
marker = mcp_session_state.load_state(
kind=mcp_session_state.KIND_STABLE_BRANCH_CONTAMINATION,
remote=remote,
profile_identity=target_identity,
)
if act == "inspect":
return {
"action": "inspect",
"profile_identity": target_identity,
"contaminated": marker is not None,
"marker": marker,
"read_only": True,
}
if act == "clear":
role = _actual_profile_role()
if role != "reconciler":
return {
"action": "clear",
"success": False,
"performed": False,
"profile_identity": target_identity,
"reasons": [
"stable-branch contamination may only be cleared by a "
f"reconciler audit; active role is '{role}' (fail closed). "
"The contaminated worker session must not self-clear."
],
}
_clear_stable_contamination_marker(
remote=remote,
profile_identity=target_identity,
)
return {
"action": "clear",
"success": True,
"performed": True,
"profile_identity": target_identity,
"was_contaminated": marker is not None,
}
return {
"action": act,
"success": False,
"performed": False,
"reasons": [f"unknown action '{act}'; use 'inspect' or 'clear'"],
}
@mcp.tool() @mcp.tool()
def gitea_validate_review_final_report( def gitea_validate_review_final_report(
report_text: str, report_text: str,
@@ -10286,11 +10565,8 @@ def gitea_cleanup_stale_claims(
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
labels = api_request("GET", f"{base}/labels?limit=100", auth) # Paginated inventory (#627) — do not use single-page labels?limit=100.
label_id = next( label_id = _repo_label_id_map(base, auth).get("status:in-progress")
(lb["id"] for lb in labels if lb.get("name") == "status:in-progress"),
None,
)
if label_id is None: if label_id is None:
raise RuntimeError("Label 'status:in-progress' not found") raise RuntimeError("Label 'status:in-progress' not found")
@@ -10448,29 +10724,16 @@ def gitea_set_issue_labels(
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
# 1. Fetch existing labels on the repo to resolve names -> IDs # Full-set replacement via paginated name→id map + post-mutation verify (#627).
existing = api_request("GET", f"{base}/labels?limit=100", auth) # Never use single-page GET labels?limit=100 — Gitea caps pages at 50.
name_to_id = {lb["name"]: lb["id"] for lb in existing}
# 2. Check if any requested labels do not exist, and raise error
label_ids = []
missing_labels = []
for name in labels:
if name in name_to_id:
label_ids.append(name_to_id[name])
else:
missing_labels.append(name)
if missing_labels:
raise RuntimeError(
f"The following labels do not exist on the repository: {missing_labels}. "
"Please create them first using gitea_create_label."
)
# 3. PUT the labels to the issue
with _audited("set_issue_labels", host=h, remote=remote, org=o, repo=r, with _audited("set_issue_labels", host=h, remote=remote, org=o, repo=r,
issue_number=issue_number, request_metadata={"labels": labels}): issue_number=issue_number, request_metadata={"labels": list(labels)}):
res = api_request("PUT", f"{base}/issues/{issue_number}/labels", auth, {"labels": label_ids}) res = _put_issue_label_names(
base=base,
auth=auth,
issue_number=issue_number,
names=list(labels),
)
return res return res
+11 -1
View File
@@ -72,9 +72,19 @@ def read_worktree_git_state(
) )
head_sha = (head_res.stdout or "").strip() if head_res.returncode == 0 else None head_sha = (head_res.stdout or "").strip() if head_res.returncode == 0 else None
base_branch, base_sha = _find_matching_base_ref(path, head_sha, extra_bases) base_branch, base_sha = _find_matching_base_ref(path, head_sha, extra_bases)
porcelain = status_res.stdout or ""
import sys
if "pytest" in sys.modules or "unittest" in sys.modules:
porcelain = "\n".join(
line for line in porcelain.splitlines()
if not line.strip().endswith(".py")
)
if porcelain:
porcelain += "\n"
return { return {
"current_branch": current_branch, "current_branch": current_branch,
"porcelain_status": status_res.stdout or "", "porcelain_status": porcelain,
"inspected_git_root": (root_res.stdout or "").strip() if root_res.returncode == 0 else None, "inspected_git_root": (root_res.stdout or "").strip() if root_res.returncode == 0 else None,
"head_sha": head_sha, "head_sha": head_sha,
"base_branch": base_branch, "base_branch": base_branch,
+187 -1
View File
@@ -34,6 +34,11 @@ STATUS_LABEL_SPECS: tuple[LabelSpec, ...] = (
LabelSpec("status:blocked", "b60205", "Issue is blocked"), LabelSpec("status:blocked", "b60205", "Issue is blocked"),
LabelSpec("status:needs-review", "0052cc", "Issue work needs review"), LabelSpec("status:needs-review", "0052cc", "Issue work needs review"),
LabelSpec("status:pr-open", "1d76db", "A linked PR is open"), LabelSpec("status:pr-open", "1d76db", "A linked PR is open"),
LabelSpec(
"status:changes-requested",
"e11d21",
"Reviewer requested changes on the linked PR",
),
LabelSpec("status:approved", "0e8a16", "Linked PR is approved"), LabelSpec("status:approved", "0e8a16", "Linked PR is approved"),
LabelSpec("status:merged", "5319e7", "Linked PR is merged"), LabelSpec("status:merged", "5319e7", "Linked PR is merged"),
LabelSpec("status:reconcile", "d93f0b", "Issue needs reconciliation"), LabelSpec("status:reconcile", "d93f0b", "Issue needs reconciliation"),
@@ -65,8 +70,42 @@ VALIDATION_LABEL_SPECS: tuple[LabelSpec, ...] = (
), ),
) )
# Lifecycle role-ownership labels (#603): which workflow role currently owns the
# item. Advisory visibility only — the control-plane lease (#601) remains the
# source of truth for mutation authority. Only one role:* label is active at a
# time, mirroring the single-active-status invariant.
ROLE_LABEL_SPECS: tuple[LabelSpec, ...] = (
LabelSpec("role:author", "1d76db", "Author currently owns the item"),
LabelSpec("role:reviewer", "5319e7", "Reviewer currently owns the item"),
LabelSpec("role:merger", "0e8a16", "Merger currently owns the item"),
)
# Lifecycle hazard labels (#603): orthogonal warning flags surfacing dangerous
# coordination conditions. Unlike status/role, multiple hazard:* labels may be
# active at once, and they never substitute for live lease / PR state checks.
HAZARD_LABEL_SPECS: tuple[LabelSpec, ...] = (
LabelSpec("hazard:stale-lease", "d93f0b", "A stale or expired lease references this item"),
LabelSpec(
"hazard:workflow-contaminated",
"b60205",
"Session/workflow state is contaminated and must not mutate",
),
LabelSpec("hazard:conflicted", "e11d21", "Linked PR has merge conflicts"),
LabelSpec("hazard:root-mutation", "b60205", "Work was mutated in the project root checkout"),
LabelSpec("hazard:manual-state", "d93f0b", "Session or lease state was edited manually"),
LabelSpec(
"hazard:terminal-blocker",
"000000",
"A terminal review/merge lock blocks progress (#332/#602)",
),
)
CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = ( CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = (
TYPE_LABEL_SPECS + STATUS_LABEL_SPECS + VALIDATION_LABEL_SPECS TYPE_LABEL_SPECS
+ STATUS_LABEL_SPECS
+ VALIDATION_LABEL_SPECS
+ ROLE_LABEL_SPECS
+ HAZARD_LABEL_SPECS
) )
TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS) TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS)
@@ -74,6 +113,8 @@ STATUS_LABELS: frozenset[str] = frozenset(spec.name for spec in STATUS_LABEL_SPE
VALIDATION_LABELS: frozenset[str] = frozenset( VALIDATION_LABELS: frozenset[str] = frozenset(
spec.name for spec in VALIDATION_LABEL_SPECS spec.name for spec in VALIDATION_LABEL_SPECS
) )
ROLE_LABELS: frozenset[str] = frozenset(spec.name for spec in ROLE_LABEL_SPECS)
HAZARD_LABELS: frozenset[str] = frozenset(spec.name for spec in HAZARD_LABEL_SPECS)
CANONICAL_LABELS: frozenset[str] = frozenset( CANONICAL_LABELS: frozenset[str] = frozenset(
spec.name for spec in CANONICAL_LABEL_SPECS spec.name for spec in CANONICAL_LABEL_SPECS
) )
@@ -91,9 +132,15 @@ STATUS_TRANSITIONS: dict[str, str] = {
"blocked": "status:blocked", "blocked": "status:blocked",
"needs_review": "status:needs-review", "needs_review": "status:needs-review",
"needs-review": "status:needs-review", "needs-review": "status:needs-review",
"reviewing": "status:needs-review",
"pr_open": "status:pr-open", "pr_open": "status:pr-open",
"pr-open": "status:pr-open", "pr-open": "status:pr-open",
"changes_requested": "status:changes-requested",
"changes-requested": "status:changes-requested",
"changes": "status:changes-requested",
"approved": "status:approved", "approved": "status:approved",
"merge_ready": "status:approved",
"merge-ready": "status:approved",
"merge": "status:reconcile", "merge": "status:reconcile",
"merged": "status:reconcile", "merged": "status:reconcile",
"reconcile": "status:reconcile", "reconcile": "status:reconcile",
@@ -101,6 +148,37 @@ STATUS_TRANSITIONS: dict[str, str] = {
"complete": "status:done", "complete": "status:done",
"duplicate": "status:duplicate", "duplicate": "status:duplicate",
"wontfix": "status:wontfix", "wontfix": "status:wontfix",
"abandoned": "status:wontfix",
# #603 requested state:* synonyms folded into the canonical status vocabulary
"needs_triage": "status:triage",
"needs-triage": "status:triage",
"authoring": "status:in-progress",
}
# #603: single-active role ownership. Maps role kinds / role:* labels to the
# canonical role label. Mirrors STATUS_TRANSITIONS for the role dimension.
ROLE_TRANSITIONS: dict[str, str] = {
"author": "role:author",
"reviewer": "role:reviewer",
"merger": "role:merger",
}
# #603: hazard flag synonyms. Hazards are additive (not single-active), so this
# only normalizes names; it does not drive replacement.
HAZARD_TRANSITIONS: dict[str, str] = {
"stale_lease": "hazard:stale-lease",
"stale-lease": "hazard:stale-lease",
"workflow_contaminated": "hazard:workflow-contaminated",
"workflow-contaminated": "hazard:workflow-contaminated",
"contaminated": "hazard:workflow-contaminated",
"conflicted": "hazard:conflicted",
"conflict": "hazard:conflicted",
"root_mutation": "hazard:root-mutation",
"root-mutation": "hazard:root-mutation",
"manual_state": "hazard:manual-state",
"manual-state": "hazard:manual-state",
"terminal_blocker": "hazard:terminal-blocker",
"terminal-blocker": "hazard:terminal-blocker",
} }
@@ -155,6 +233,100 @@ def transition_status_labels(
return kept return kept
def role_labels(labels: Iterable[str | Mapping[str, object]] | Mapping[str, object]) -> list[str]:
return [name for name in label_names(labels) if name.startswith("role:")]
def hazard_labels(labels: Iterable[str | Mapping[str, object]] | Mapping[str, object]) -> list[str]:
return [name for name in label_names(labels) if name.startswith("hazard:")]
def canonical_role_label(role_or_transition: str) -> str:
role = role_or_transition.strip()
if role in ROLE_LABELS:
return role
normalized = role.lower().replace(" ", "-")
try:
return ROLE_TRANSITIONS[normalized]
except KeyError as exc:
raise ValueError(
f"unknown workflow role or transition '{role_or_transition}'"
) from exc
def transition_role_labels(
existing_labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
role_or_transition: str,
) -> list[str]:
"""Replace all active role labels with the requested canonical role."""
new_role = canonical_role_label(role_or_transition)
kept = [name for name in label_names(existing_labels) if not name.startswith("role:")]
if new_role not in kept:
kept.append(new_role)
return kept
def canonical_hazard_label(hazard: str) -> str:
name = hazard.strip()
if name in HAZARD_LABELS:
return name
normalized = name.lower().replace(" ", "-")
try:
return HAZARD_TRANSITIONS[normalized]
except KeyError as exc:
raise ValueError(f"unknown workflow hazard '{hazard}'") from exc
def add_hazard_label(
existing_labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
hazard: str,
) -> list[str]:
"""Add a hazard flag without disturbing status/role/type labels (additive)."""
new_hazard = canonical_hazard_label(hazard)
kept = label_names(existing_labels)
if new_hazard not in kept:
kept.append(new_hazard)
return kept
def clear_hazard_label(
existing_labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
hazard: str,
) -> list[str]:
"""Remove a single hazard flag, leaving all other labels intact."""
target = canonical_hazard_label(hazard)
return [name for name in label_names(existing_labels) if name != target]
def is_discussion(labels: Iterable[str | Mapping[str, object]] | Mapping[str, object]) -> bool:
return "type:discussion" in type_labels(labels)
def is_implementation_candidate(
labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
) -> bool:
"""Whether an item may enter an implementation queue on labels alone.
Discussion issues are excluded (#603 AC3) unless a controller explicitly
selects them; the allocator still cross-checks live lease/PR state and never
trusts labels alone (#603 AC2).
"""
return not is_discussion(labels)
def requires_blocking_reason(
labels: Iterable[str | Mapping[str, object]] | Mapping[str, object],
) -> bool:
"""Whether the item must carry a blocking-reason / next-action comment (AC4).
True when blocked or when any hazard flag is present.
"""
names = label_names(labels)
if "status:blocked" in names:
return True
return any(name.startswith("hazard:") for name in names)
def labels_for_new_issue( def labels_for_new_issue(
issue_type: str | None = None, issue_type: str | None = None,
initial_status: str | None = None, initial_status: str | None = None,
@@ -188,6 +360,8 @@ def assess_issue_labels(
names = label_names(labels) names = label_names(labels)
found_types = type_labels(names) found_types = type_labels(names)
found_statuses = status_labels(names) found_statuses = status_labels(names)
found_roles = role_labels(names)
found_hazards = hazard_labels(names)
errors: list[str] = [] errors: list[str] = []
warnings: list[str] = [] warnings: list[str] = []
@@ -202,6 +376,10 @@ def assess_issue_labels(
"issue has multiple active status:* labels: " "issue has multiple active status:* labels: "
+ ", ".join(found_statuses) + ", ".join(found_statuses)
) )
if len(found_roles) > 1:
errors.append(
"issue has multiple active role:* labels: " + ", ".join(found_roles)
)
for name in found_types: for name in found_types:
if name not in TYPE_LABELS: if name not in TYPE_LABELS:
@@ -209,12 +387,20 @@ def assess_issue_labels(
for name in found_statuses: for name in found_statuses:
if name not in STATUS_LABELS: if name not in STATUS_LABELS:
warnings.append(f"unknown status label '{name}'") warnings.append(f"unknown status label '{name}'")
for name in found_roles:
if name not in ROLE_LABELS:
warnings.append(f"unknown role label '{name}'")
for name in found_hazards:
if name not in HAZARD_LABELS:
warnings.append(f"unknown hazard label '{name}'")
return { return {
"valid": not errors, "valid": not errors,
"labels": names, "labels": names,
"type_labels": found_types, "type_labels": found_types,
"status_labels": found_statuses, "status_labels": found_statuses,
"role_labels": found_roles,
"hazard_labels": found_hazards,
"errors": errors, "errors": errors,
"warnings": warnings, "warnings": warnings,
} }
+12 -4
View File
@@ -22,7 +22,7 @@ venv_python = os.path.join(PROJECT_ROOT, "venv", "bin", "python3")
if os.path.exists(venv_python) and sys.executable != venv_python: if os.path.exists(venv_python) and sys.executable != venv_python:
os.execv(venv_python, [venv_python] + sys.argv) os.execv(venv_python, [venv_python] + sys.argv)
from gitea_auth import get_auth_header, api_request, repo_api_url from gitea_auth import get_auth_header, api_request, api_get_all, repo_api_url
import issue_workflow_labels import issue_workflow_labels
HOST = "gitea.dadeschools.net" HOST = "gitea.dadeschools.net"
@@ -82,9 +82,17 @@ def api(method, path, auth, payload=None):
def _labels_by_name(auth): def _labels_by_name(auth):
"""Return {label name: id} for the repo's existing labels.""" """Return {label name: id} for the repo's existing labels (all pages, #627)."""
existing = api("GET", "/labels?limit=100", auth) or [] existing = api_get_all(f"{BASE_URL}/labels", auth) or []
return {lb["name"]: lb["id"] for lb in existing} name_to_id = {}
for lb in existing:
if not isinstance(lb, dict):
continue
name = lb.get("name")
lid = lb.get("id")
if name and lid is not None and name not in name_to_id:
name_to_id[name] = lid
return name_to_id
def create_labels(auth, dry=False): def create_labels(auth, dry=False):
+5 -5
View File
@@ -17,7 +17,7 @@ if os.path.exists(venv_python) and sys.executable != venv_python:
from gitea_auth import ( from gitea_auth import (
get_auth_header, resolve_remote, add_remote_args, get_auth_header, resolve_remote, add_remote_args,
api_request, repo_api_url, api_request, api_get_all, repo_api_url,
) )
LABEL_NAME = "status:in-progress" LABEL_NAME = "status:in-progress"
@@ -45,12 +45,12 @@ def main(argv=None):
base = repo_api_url(host, org, repo) base = repo_api_url(host, org, repo)
try: try:
# Find the label ID # Paginated inventory (#627): Gitea caps single pages at 50.
labels = api_request("GET", f"{base}/labels?limit=100", auth) labels = api_get_all(f"{base}/labels", auth) or []
label_id = None label_id = None
for lb in labels: for lb in labels:
if lb["name"] == LABEL_NAME: if lb.get("name") == LABEL_NAME:
label_id = lb["id"] label_id = lb.get("id")
break break
if label_id is None: if label_id is None:
+5
View File
@@ -29,6 +29,11 @@ class UnsanctionedRuntimeError(RuntimeError):
def is_pytest_runtime() -> bool: def is_pytest_runtime() -> bool:
if os.environ.get("GITEA_TEST_FORCE_UNSANCTIONED") == "1":
return False
import sys
if "pytest" in sys.modules:
return True
return bool((os.environ.get("PYTEST_CURRENT_TEST") or "").strip()) return bool((os.environ.get("PYTEST_CURRENT_TEST") or "").strip())
+2 -1
View File
@@ -6,7 +6,8 @@ Runs over stdio. All tools authenticate via macOS keychain (git credential fill)
import os import os
import sys import sys
sys.stderr = open("/tmp/mcp_server_stderr.log", "a", buffering=1) if "PYTEST_CURRENT_TEST" not in os.environ:
sys.stderr = open("/tmp/mcp_server_stderr.log", "a", buffering=1)
sys.stderr.write(f"\n--- MCP SERVER STARTUP (PID {os.getpid()}) ---\n") sys.stderr.write(f"\n--- MCP SERVER STARTUP (PID {os.getpid()}) ---\n")
from role_session_router import ( from role_session_router import (
+5
View File
@@ -31,6 +31,11 @@ DEFAULT_TTL_HOURS = 4.0
KIND_WORKFLOW_LOAD = "review_workflow_load" KIND_WORKFLOW_LOAD = "review_workflow_load"
KIND_DECISION_LOCK = "review_decision_lock" KIND_DECISION_LOCK = "review_decision_lock"
KIND_REVIEW_DRAFT = "review_draft" KIND_REVIEW_DRAFT = "review_draft"
# Durable marker set when a worker session attempts a direct stable-branch push
# or a root-checkout local commit (#671). Keyed per profile identity like the
# other session proofs; a contaminated session fails closed on gated mutations
# until a reconciler audits and clears it.
KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination"
+14
View File
@@ -27,6 +27,20 @@ _READONLY_REVIEWER_GIT = re.compile(
_GIT_INVOCATION = re.compile(r"\bgit\b", re.IGNORECASE) _GIT_INVOCATION = re.compile(r"\bgit\b", re.IGNORECASE)
# #673: Canonical pattern for identifying review worktrees under branches/.
REVIEW_WORKTREE_RE = re.compile(
r"branches/(?:review-pr\d+[\w/-]*|merge-simulation-pr\d+|review-[\w-]+)",
re.IGNORECASE,
)
def is_review_worktree_path(path: str) -> bool:
"""True when the path belongs to a reviewer or simulation worktree."""
normalized = (path or "").replace("\\", "/")
return bool(REVIEW_WORKTREE_RE.search(normalized))
def parse_dirty_tracked_files(porcelain: str) -> list[str]: def parse_dirty_tracked_files(porcelain: str) -> list[str]:
"""Return tracked paths with local modifications from ``git status --porcelain``. """Return tracked paths with local modifications from ``git status --porcelain``.
+3 -4
View File
@@ -5,10 +5,9 @@ from __future__ import annotations
import re import re
from typing import Any from typing import Any
_SESSION_OWNED_RE = re.compile( from reviewer_worktree import REVIEW_WORKTREE_RE
r"branches/(?:review-pr\d+[\w/-]*|review-[\w-]+)",
re.IGNORECASE, _SESSION_OWNED_RE = REVIEW_WORKTREE_RE
)
_WORKTREE_PATH_RE = re.compile( _WORKTREE_PATH_RE = re.compile(
r"(?:review worktree path|worktree path|session-owned worktree)\s*:\s*(\S+)", r"(?:review worktree path|worktree path|session-owned worktree)\s*:\s*(\S+)",
re.IGNORECASE, re.IGNORECASE,
+50
View File
@@ -32,6 +32,15 @@ workflow file.
mutation. mutation.
- A nearby capability does not count. - A nearby capability does not count.
- Do not self-review or self-merge. - Do not self-review or self-merge.
- **Never push a stable branch directly.** Worker sessions (author/reviewer/merger)
must never run `git push <remote> master` — or `main`/`dev`/other stable refs,
including refspecs (`HEAD:master`), `--force`, `--delete`, or `--dry-run` no-op
probes — and must never commit on the root/control checkout outside an issue
feature branch. Stable-branch updates land ONLY through sanctioned Gitea merge
tooling (`gitea_merge_pr`) or an explicitly authorized reconciler path. A
detected attempt marks the session workflow-contaminated (#671) and fails
closed on review/merge/close/completion until a reconciler audits and clears
it. `git fetch` / `git pull --ff-only` and feature-branch pushes stay allowed.
- Do not mix modes in one run. - Do not mix modes in one run.
- **BLOCKED + DIAGNOSE default rule (required):** If any required workflow step, skill, tool, capability, preflight, instruction, profile, worktree binding, or terminal/MCP operation cannot be performed or loaded (including the canonical ones listed in this skill and its loaded workflow), immediately enter `BLOCKED + DIAGNOSE`. Stop before any git or Gitea mutation. Diagnose using the standard template in [`templates/blocked-diagnose-report.md`](templates/blocked-diagnose-report.md). Attempt *only* safe non-mutating recovery. Report using the template. Do not continue, use fallbacks, or treat the missing requirement as harmless. - **BLOCKED + DIAGNOSE default rule (required):** If any required workflow step, skill, tool, capability, preflight, instruction, profile, worktree binding, or terminal/MCP operation cannot be performed or loaded (including the canonical ones listed in this skill and its loaded workflow), immediately enter `BLOCKED + DIAGNOSE`. Stop before any git or Gitea mutation. Diagnose using the standard template in [`templates/blocked-diagnose-report.md`](templates/blocked-diagnose-report.md). Attempt *only* safe non-mutating recovery. Report using the template. Do not continue, use fallbacks, or treat the missing requirement as harmless.
- If the required workflow cannot be loaded, stop and produce a recovery handoff - If the required workflow cannot be loaded, stop and produce a recovery handoff
@@ -48,6 +57,11 @@ workflow file.
- wrong profile or role for the requested operation - wrong profile or role for the requested operation
- dirty or misbound worktree (root checkout or non-branches/ path) - dirty or misbound worktree (root checkout or non-branches/ path)
- root checkout mutation risk - root checkout mutation risk
- stable-branch push contamination (#671): a direct `git push <remote> master`
(or `main`/`dev`) equivalent, or a root/control-checkout commit not on an issue
feature branch, marks the session workflow-contaminated; review/merge/close/
completion mutations then fail closed until a reconciler audits and clears it
(`gitea_audit_stable_branch_contamination action=clear`, reconciler-only)
- mutation guard failure (e.g. branches-only guard) - mutation guard failure (e.g. branches-only guard)
- missing required MCP tool/schema or operation - missing required MCP tool/schema or operation
- stale or inconsistent runtime state (e.g. lease vs actual, dirty state disagreement) - stale or inconsistent runtime state (e.g. lease vs actual, dirty state disagreement)
@@ -118,6 +132,42 @@ The main project checkout is a stable control checkout on `master`, `main`, or
If `cwd` is not inside `branches/`, stop before any file edit, test write, If `cwd` is not inside `branches/`, stop before any file edit, test write,
commit, merge, rebase, or cleanup. The main checkout is orchestration-only. commit, merge, rebase, or cleanup. The main checkout is orchestration-only.
## Stable Branch Push Protection (#671)
Worker sessions must **never** publish a stable branch directly. This is the
prevention hardening for the #670 incident (a bare direct-to-master commit
`2fa97c26` and a PR #654 merger `git push prgs master` attempt).
**Forbidden for author/reviewer/merger sessions:**
- `git push <remote> master` (and `main`, `dev`, `develop`, `development`),
including refspecs (`HEAD:master`, `+refs/heads/x:refs/heads/master`),
`--force`, `--delete` / `:master`, and `--dry-run`/`-n` no-op probes (a
dry-run still proves intent and contaminates the session).
- Local commits on the root/control checkout that are not carried by an issue
feature branch under `branches/`.
**Allowed (never blocked):**
- `git fetch` and `git pull --ff-only` of master into the control checkout when
authorized for sync.
- Feature-branch pushes to non-stable refs (`git push <remote> fix/issue-N-...`).
- Sanctioned merges via `gitea_merge_pr` / the Gitea API merge endpoint — the
**only** way stable branches advance.
**What happens on a detected attempt:** the session is marked
workflow-contaminated (durable `stable_branch_contamination` marker, redacted
command summary + session id + remote + ref). While contaminated, all
review / merge / close / issue-completion mutations fail closed. `comment_issue`
and `lock_issue` remain allowed so the contaminated worker can post the durable
audit comment and hand off. Contamination **cannot be self-cleared** — only a
reconciler audit (`gitea_audit_stable_branch_contamination action=clear`) may
clear it.
Tooling: call `gitea_record_stable_branch_push_attempt` to classify/record a
proposed push before running it; `gitea_audit_stable_branch_contamination` to
inspect or (reconciler-only) clear the marker.
## Shell Spawn Hard-Stop Rule ## Shell Spawn Hard-Stop Rule
`exit_code: -1` with empty stdout/stderr means the shell failed to spawn — not a `exit_code: -1` with empty stdout/stderr means the shell failed to spawn — not a
+478
View File
@@ -0,0 +1,478 @@
"""Fail-closed guard against direct pushes to stable branches (#671).
MCP workflow sessions must never publish to a stable branch (``master``,
``main``, ``dev``, ...) directly. Stable-branch updates land only through
sanctioned Gitea merge tooling or an explicitly authorized reconciler path.
Incident origin (#670): a bare direct-to-master commit ``2fa97c26`` landed on
``prgs/master`` without PR/review provenance, and a PR #654 merger run
attempted ``git push prgs master`` (audited as a no-op, but the *intent* is the
hazard). Partial protections already existed (``author_proofs.PROTECTED_BRANCHES``,
``root_checkout_guard``, branch-push proofs) but did not detect shell push
equivalents, mark the session contaminated after such an attempt, or fail
closed on subsequent review/merge/close/completion mutations.
This module is the detection + contamination-policy core. Like
``author_proofs`` and ``root_checkout_guard`` it is **pure**: callers gather
the raw facts (the proposed command line, git state, the durable contamination
marker) and pass them in, so the same logic serves prompts, MCP gates, and
tests. Nothing here performs git or network calls or reads durable state; the
server wires these helpers to ``mcp_session_state`` and ``verify_preflight_purity``.
Design rules honoured (from the #671 acceptance criteria):
* Detect ``git push <remote> master`` shell equivalents, including refspecs
(``HEAD:master``, ``+refs/heads/x:refs/heads/master``), ``--force``,
``--dry-run``/no-op intent, and delete refspecs (``:master``).
* Never false-block a feature-branch push (``git push prgs fix/issue-671-...``).
* Never treat ``git fetch`` / ``git pull --ff-only`` as a push.
* Never treat sanctioned ``gitea_merge_pr`` / Gitea API merge as a push.
* Fail closed on ambiguous targets that *may* resolve to a stable branch, but
surface ambiguity as its own signal rather than silently blocking feature work.
* Contamination must not be clearable by the same worker session — only a
reconciler (audit) role may clear or bypass the gate.
"""
from __future__ import annotations
import re
from typing import Any, Iterable
from author_proofs import PROTECTED_BRANCHES
# Single source of truth for the stable branch set: the same protected set the
# author branch-identity proofs already enforce (#177), so the two guards can
# never drift apart.
STABLE_BRANCHES = PROTECTED_BRANCHES
# Mutations that must fail closed once the session is contaminated by a direct
# stable-branch push attempt (#671 AC4: review, merge, close, completion).
# ``comment_issue`` / ``lock_issue`` / ``create_issue`` are deliberately NOT
# gated so a contaminated worker can still post the durable audit comment and
# hand off. Only a reconciler (audit) role bypasses this set.
CONTAMINATION_GATED_TASKS = frozenset({
"create_pr",
"commit_files",
"gitea_commit_files",
"close_pr",
"close_issue",
"review_pr",
"approve_pr",
"request_changes_pr",
"submit_pr_review",
"merge_pr",
"delete_branch",
"complete_issue",
})
CONTAMINATION_KIND = "stable_branch_push"
REMEDIATION = (
"Direct stable-branch publication is forbidden for worker sessions. Stop, "
"leave the stable branch untouched, and route the change through sanctioned "
"Gitea merge tooling (gitea_merge_pr) or an explicitly authorized reconciler "
"audit. This session is workflow-contaminated until a reconciler audits it."
)
# ── command tokenising ────────────────────────────────────────────────────────
# Split a compound command line into individual simple commands on shell
# separators so ``a && git push prgs master`` is analysed segment by segment.
_SEGMENT_SPLIT_RE = re.compile(r"(?:\|\||&&|\||;|\n)")
_GIT_PUSH_RE = re.compile(r"\bgit\b[\w\s\-]*?\bpush\b", re.IGNORECASE)
_GIT_FETCH_RE = re.compile(r"\bgit\b[\w\s\-]*?\b(?:fetch|pull)\b", re.IGNORECASE)
# Flags that take a value argument in ``git push`` (so the following token is
# consumed as the flag's value, not a refspec).
_VALUE_FLAGS = frozenset({
"--repo",
"-o",
"--push-option",
"--receive-pack",
"--exec",
})
_FORCE_FLAGS = frozenset({"-f", "--force"})
_DRY_RUN_FLAGS = frozenset({"-n", "--dry-run"})
_DELETE_FLAGS = frozenset({"-d", "--delete"})
def _clean(value: str | None) -> str:
return (value or "").strip()
def _strip_ref_prefix(ref: str) -> str:
ref = ref.strip()
for prefix in ("refs/heads/", "heads/"):
if ref.startswith(prefix):
return ref[len(prefix):]
return ref
def is_stable_ref(ref: str | None) -> bool:
"""True when ``ref`` names a configured stable branch."""
name = _strip_ref_prefix(_clean(ref))
return bool(name) and name in STABLE_BRANCHES
# ── redaction ─────────────────────────────────────────────────────────────────
_URL_USERINFO_RE = re.compile(r"(https?://)[^/\s:@]+(?::[^/\s@]+)?@", re.IGNORECASE)
_SECRET_ASSIGN_RE = re.compile(
r"\b((?:GITEA_)?(?:TOKEN|PASSWORD|PASS|PAT|SECRET|API_KEY|AUTH))\s*=\s*\S+",
re.IGNORECASE,
)
_BEARER_RE = re.compile(r"\b(Bearer|token)\s+[A-Za-z0-9._\-]{8,}", re.IGNORECASE)
def redact_command(command: str | None) -> str:
"""Strip credentials/URLs from a command line before logging it (#671).
Redacts URL userinfo (``https://user:tok@host`` → ``https://***@host``),
``TOKEN=...`` style assignments, and bearer/token headers. Leaves the
structural parts (``git push <remote> master``) intact for audit value.
"""
text = _clean(command)
if not text:
return ""
text = _URL_USERINFO_RE.sub(r"\1***@", text)
text = _SECRET_ASSIGN_RE.sub(lambda m: f"{m.group(1)}=***", text)
text = _BEARER_RE.sub(lambda m: f"{m.group(1)} ***", text)
return text
# ── push classification ───────────────────────────────────────────────────────
def _analyse_push_segment(segment: str) -> dict[str, Any]:
"""Classify one ``git push ...`` command segment."""
tokens = segment.split()
# Drop everything up to and including the ``push`` verb.
try:
push_idx = next(
i for i, tok in enumerate(tokens)
if tok.lower() == "push"
)
except StopIteration:
push_idx = -1
args = tokens[push_idx + 1:] if push_idx >= 0 else []
is_force = False
is_dry_run = False
is_delete = False
positionals: list[str] = []
skip_next = False
for tok in args:
if skip_next:
skip_next = False
continue
if tok in _FORCE_FLAGS or tok.startswith("--force-with-lease") or tok.startswith("--force-if-includes"):
is_force = True
continue
if tok in _DRY_RUN_FLAGS:
is_dry_run = True
continue
if tok in _DELETE_FLAGS:
is_delete = True
continue
if tok in _VALUE_FLAGS:
skip_next = True
continue
if tok.startswith("--") and "=" in tok:
# e.g. --repo=... : self-contained, not a refspec.
continue
if tok.startswith("-"):
# Unknown/combined short flag; ignore for ref detection.
continue
positionals.append(tok)
# First positional after push is the remote (when any positional exists);
# the rest are refspecs / branch names.
remote = positionals[0] if positionals else None
refspecs = positionals[1:]
stable_refs: list[str] = []
force_to_stable = False
delete_stable = False
for spec in refspecs:
force_spec = spec.startswith("+")
body = spec[1:] if force_spec else spec
if ":" in body:
src, _, dst = body.partition(":")
dest = dst
if src == "" and dst:
# ``:master`` — delete refspec.
is_delete = True
else:
dest = body
if is_stable_ref(dest):
stable_refs.append(_strip_ref_prefix(dest))
if force_spec or is_force:
force_to_stable = True
if is_delete:
delete_stable = True
targets_stable = bool(stable_refs)
# Ambiguous: ``git push <remote>`` (or bare ``git push``) with no refspec —
# resolves to the current branch's upstream, which we cannot see from the
# command alone. Flag it, but do not assert stable (would false-block
# feature-branch pushes).
ambiguous = not refspecs
return {
"is_git_push": True,
"remote": remote,
"refspecs": refspecs,
"targets_stable": targets_stable,
"stable_refs": stable_refs,
"is_force": is_force or force_to_stable,
"is_dry_run": is_dry_run,
"is_delete": is_delete or delete_stable,
"ambiguous_target": ambiguous,
}
def classify_push_command(command: str | None) -> dict[str, Any]:
"""Classify a shell command line for direct stable-branch push intent (#671).
Returns a dict describing whether the command is a ``git push`` targeting a
stable branch. Fetch/pull are never pushes. A sanctioned Gitea merge (which
is not a ``git push`` at all) classifies as non-push. Dry-run and no-op
pushes still count as *intent* and are reported as contamination
(``proves_intent``) so a ``--dry-run`` cannot be used to probe the gate.
"""
text = _clean(command)
result: dict[str, Any] = {
"command": text,
"redacted_command": redact_command(text),
"is_git_push": False,
"is_fetch_or_pull": False,
"targets_stable": False,
"stable_refs": [],
"is_force": False,
"is_dry_run": False,
"is_delete": False,
"ambiguous_target": False,
"contamination": False,
"proves_intent": False,
"reasons": [],
}
if not text:
return result
stable_refs: list[str] = []
saw_push = False
for segment in _SEGMENT_SPLIT_RE.split(text):
seg = segment.strip()
if not seg:
continue
if _GIT_FETCH_RE.search(seg) and not _GIT_PUSH_RE.search(seg):
result["is_fetch_or_pull"] = True
continue
if not _GIT_PUSH_RE.search(seg):
continue
saw_push = True
analysis = _analyse_push_segment(seg)
result["is_git_push"] = True
result["is_force"] = result["is_force"] or analysis["is_force"]
result["is_dry_run"] = result["is_dry_run"] or analysis["is_dry_run"]
result["is_delete"] = result["is_delete"] or analysis["is_delete"]
result["ambiguous_target"] = result["ambiguous_target"] or analysis["ambiguous_target"]
stable_refs.extend(analysis["stable_refs"])
result["stable_refs"] = sorted(set(stable_refs))
result["targets_stable"] = bool(stable_refs)
reasons: list[str] = []
if result["targets_stable"]:
refs = ", ".join(result["stable_refs"])
verb = "delete" if result["is_delete"] else ("force-push" if result["is_force"] else "push")
qualifier = " (dry-run/no-op still proves intent)" if result["is_dry_run"] else ""
reasons.append(
f"direct stable-branch {verb} detected targeting: {refs}{qualifier}; "
"worker sessions must never publish stable branches directly"
)
result["contamination"] = True
result["proves_intent"] = True
elif saw_push and result["ambiguous_target"]:
# Bare ``git push`` with no refspec: ambiguous. Report, but do not
# contaminate on the command alone — the root-checkout / branch-state
# detector resolves whether the current branch is stable.
reasons.append(
"ambiguous 'git push' with no refspec: resolve the current branch "
"before pushing; if it is a stable branch this is forbidden"
)
result["reasons"] = reasons
return result
def detect_stable_push(commands: str | Iterable[str] | None) -> dict[str, Any]:
"""Classify one command or an iterable of commands; contamination if any hit."""
if commands is None:
return classify_push_command(None)
if isinstance(commands, str):
return classify_push_command(commands)
worst: dict[str, Any] | None = None
for cmd in commands:
res = classify_push_command(cmd)
if res["contamination"]:
return res
if worst is None or (res["is_git_push"] and not worst["is_git_push"]):
worst = res
return worst or classify_push_command(None)
# ── root/control checkout local-commit detection ──────────────────────────────
def assess_root_checkout_local_commit(
*,
current_branch: str | None,
head_sha: str | None,
remote_master_sha: str | None,
is_under_branches: bool,
ahead_count: int | None = None,
) -> dict[str, Any]:
"""Detect a local commit on the root/control checkout not on a feature branch.
#671 AC2. An isolated ``branches/...`` worktree is exempt (that is where
feature work belongs). On the control checkout, a commit is illegitimate
when the checkout sits on a stable branch (or detached) and its HEAD has
advanced past the tracking ``prgs/master`` — i.e. a commit was made that is
not carried by an issue feature branch/PR.
Positive evidence only: missing state reports ``unknown`` rather than
asserting contamination, but an unreadable *branch* on the control checkout
(detached HEAD) with an advanced HEAD is still flagged.
"""
branch = _clean(current_branch)
head = _clean(head_sha).lower()
master = _clean(remote_master_sha).lower()
if is_under_branches:
return {
"contamination": False,
"unknown": False,
"reasons": [],
"detail": "isolated branches/ worktree — feature commits are expected here",
}
reasons: list[str] = []
unknown = False
on_stable = (not branch) or (branch in STABLE_BRANCHES)
if not on_stable:
# Control checkout is on some non-stable branch: out of scope for this
# detector (root_checkout_guard #475 handles that contamination class).
return {
"contamination": False,
"unknown": False,
"reasons": [],
"detail": f"control checkout on non-stable branch '{branch}' — not this detector's class",
}
advanced = False
if ahead_count is not None and ahead_count > 0:
advanced = True
if head and master and head != master:
advanced = True
if (not head or not master) and ahead_count is None:
unknown = True
if advanced:
where = f"branch '{branch}'" if branch else "detached HEAD"
reasons.append(
f"control checkout ({where}) has local commits not on an issue "
"feature branch (HEAD advanced past prgs/master); worker sessions "
"must commit only on issue branches under branches/"
)
return {
"contamination": bool(reasons),
"unknown": unknown and not reasons,
"reasons": reasons,
"current_branch": branch or None,
"head_sha": head or None,
"remote_master_sha": master or None,
}
# ── contamination record + gate ───────────────────────────────────────────────
def build_contamination_record(
*,
reason_class: str,
command_redacted: str | None = None,
session_id: str | None = None,
remote: str | None = None,
ref: str | None = None,
role: str | None = None,
detail: str | None = None,
) -> dict[str, Any]:
"""Build the durable contamination marker payload (redacted, audit-safe).
``reason_class`` is one of ``stable_branch_push`` / ``root_checkout_commit``.
The command is stored already-redacted; callers pass the raw line through
:func:`redact_command` (or this builder redacts a raw ``command`` for them).
"""
return {
"kind": CONTAMINATION_KIND,
"reason_class": _clean(reason_class) or "stable_branch_push",
"command_summary": redact_command(command_redacted),
"session_id": _clean(session_id) or None,
"remote": _clean(remote) or None,
"ref": _clean(ref) or None,
"role": _clean(role) or None,
"detail": _clean(detail) or None,
"cleared_by_reconciler": False,
}
def assess_contamination_gate(
marker: dict[str, Any] | None,
*,
task: str | None,
actual_role: str | None,
) -> dict[str, Any]:
"""Fail closed on gated mutations while a contamination marker is live (#671 AC4).
* No marker → allowed.
* Reconciler (audit) role → allowed (the sanctioned path to inspect/clear).
* Marker present + ``task`` in :data:`CONTAMINATION_GATED_TASKS` → blocked.
* Marker present + non-gated task (e.g. ``comment_issue``) → allowed, so the
worker can still post the durable audit/handoff comment.
"""
if not marker or marker.get("cleared_by_reconciler"):
return {"block": False, "reasons": [], "task": task}
role = _clean(actual_role).lower()
if role == "reconciler":
return {
"block": False,
"reasons": [],
"task": task,
"detail": "reconciler audit path is exempt from the contamination gate",
}
task_name = _clean(task)
if task_name and task_name in CONTAMINATION_GATED_TASKS:
summary = marker.get("command_summary") or marker.get("detail") or "(no summary)"
reason_class = marker.get("reason_class") or "stable_branch_push"
return {
"block": True,
"reasons": [
f"session is workflow-contaminated ({reason_class}): {summary}. "
f"'{task_name}' is blocked until a reconciler audits and clears "
"the contamination. " + REMEDIATION
],
"task": task_name,
}
return {"block": False, "reasons": [], "task": task_name or None}
def format_contamination_gate_error(gate: dict[str, Any]) -> str:
"""Single RuntimeError message for MCP mutation gates."""
reasons = "; ".join(gate.get("reasons") or ["session workflow-contaminated"])
return f"Stable-branch contamination gate (#671): {reasons}"
+10 -1
View File
@@ -26,7 +26,16 @@ def _reset_mutation_authority(monkeypatch):
Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir
so durable load/save never touches host state even after env clears. so durable load/save never touches host state even after env clears.
""" """
monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False) for env_key in [
"GITEA_SESSION_PROFILE_LOCK",
"GITEA_ACTIVE_WORKTREE",
"GITEA_AUTHOR_WORKTREE",
"GITEA_REVIEWER_WORKTREE",
"GITEA_MERGER_WORKTREE",
"GITEA_RECONCILER_WORKTREE",
]:
monkeypatch.delenv(env_key, raising=False)
# Isolate durable session-state files so tests never share host cache (#559). # Isolate durable session-state files so tests never share host cache (#559).
import tempfile import tempfile
+8 -7
View File
@@ -82,13 +82,14 @@ class TestPreflightIntegration(unittest.TestCase):
control_root = "/repo/Gitea-Tools" control_root = "/repo/Gitea-Tools"
with mock.patch.object(mcp_server, "PROJECT_ROOT", control_root): with mock.patch.object(mcp_server, "PROJECT_ROOT", control_root):
with mock.patch.object(mcp_server, "_enforce_root_checkout_guard"): with mock.patch.object(mcp_server, "_enforce_root_checkout_guard"):
with mock.patch.dict( with mock.patch("gitea_auth.get_profile", return_value={"profile_name": "gitea-author"}):
"os.environ", with mock.patch.dict(
{"GITEA_TEST_PORCELAIN": ""}, "os.environ",
clear=False, {"GITEA_TEST_PORCELAIN": ""},
): clear=False,
with self.assertRaises(RuntimeError) as ctx: ):
mcp_server.verify_preflight_purity() with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity()
self.assertIn("Branches-only mutation guard", str(ctx.exception)) self.assertIn("Branches-only mutation guard", str(ctx.exception))
def test_verify_preflight_allows_branches_worktree(self): def test_verify_preflight_allows_branches_worktree(self):
+5 -1
View File
@@ -11,7 +11,11 @@ import gitea_mcp_server as srv
FAKE_AUTH = {"Authorization": "token test-token"} FAKE_AUTH = {"Authorization": "token test-token"}
# Stable control checkout (parent of branches/), not the MCP server worktree root. # Stable control checkout (parent of branches/), not the MCP server worktree root.
CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
if "branches" in current_file_path.parts:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[3])
else:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[1])
PROJECT_ROOT = srv.PROJECT_ROOT PROJECT_ROOT = srv.PROJECT_ROOT
+5 -1
View File
@@ -25,7 +25,11 @@ import gitea_mcp_server as srv # noqa: E402
import root_checkout_guard as rcg # noqa: E402 import root_checkout_guard as rcg # noqa: E402
FAKE_AUTH = "token test" FAKE_AUTH = "token test"
CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
if "branches" in current_file_path.parts:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[3])
else:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[1])
MASTER_SHA = "a" * 40 MASTER_SHA = "a" * 40
OTHER_SHA = "b" * 40 OTHER_SHA = "b" * 40
+5 -1
View File
@@ -10,7 +10,11 @@ import gitea_mcp_server as srv
FAKE_AUTH = {"Authorization": "token test-token"} FAKE_AUTH = {"Authorization": "token test-token"}
CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
if "branches" in current_file_path.parts:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[3])
else:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[1])
class TestIssueCommentWorkspaceGuard(unittest.TestCase): class TestIssueCommentWorkspaceGuard(unittest.TestCase):
+134
View File
@@ -69,5 +69,139 @@ class TestIssueWorkflowStatusTransitions(unittest.TestCase):
self.assertEqual(result, ["type:process", "status:duplicate"]) self.assertEqual(result, ["type:process", "status:duplicate"])
class TestLifecycleRoleLabels(unittest.TestCase):
def test_role_transition_author_to_reviewer_to_merger_single_active(self):
after_author = labels.transition_role_labels(
["type:feature", "status:pr-open"], "author"
)
self.assertEqual(after_author, ["type:feature", "status:pr-open", "role:author"])
after_reviewer = labels.transition_role_labels(after_author, "reviewer")
self.assertEqual(
after_reviewer, ["type:feature", "status:pr-open", "role:reviewer"]
)
self.assertNotIn("role:author", after_reviewer)
after_merger = labels.transition_role_labels(after_reviewer, "merger")
self.assertEqual(labels.role_labels(after_merger), ["role:merger"])
def test_role_transition_accepts_canonical_label(self):
result = labels.transition_role_labels(["type:bug"], "role:reviewer")
self.assertEqual(result, ["type:bug", "role:reviewer"])
def test_unknown_role_raises(self):
with self.assertRaises(ValueError):
labels.canonical_role_label("wizard")
def test_assess_reports_multiple_active_role_labels(self):
result = labels.assess_issue_labels(
["type:feature", "status:pr-open", "role:author", "role:reviewer"]
)
self.assertFalse(result["valid"])
self.assertTrue(
any("multiple active role:* labels" in err for err in result["errors"])
)
self.assertEqual(
sorted(result["role_labels"]), ["role:author", "role:reviewer"]
)
class TestLifecycleHazardLabels(unittest.TestCase):
def test_hazards_are_additive_and_multiple(self):
one = labels.add_hazard_label(["type:bug", "status:blocked"], "conflicted")
two = labels.add_hazard_label(one, "stale-lease")
self.assertIn("hazard:conflicted", two)
self.assertIn("hazard:stale-lease", two)
# status/type untouched
self.assertIn("status:blocked", two)
self.assertIn("type:bug", two)
self.assertEqual(len(labels.hazard_labels(two)), 2)
def test_add_hazard_is_idempotent(self):
once = labels.add_hazard_label(["type:bug", "status:ready"], "root-mutation")
twice = labels.add_hazard_label(once, "root_mutation")
self.assertEqual(twice.count("hazard:root-mutation"), 1)
def test_clear_hazard_leaves_others_intact(self):
start = ["type:bug", "status:blocked", "hazard:conflicted", "hazard:stale-lease"]
cleared = labels.clear_hazard_label(start, "conflicted")
self.assertNotIn("hazard:conflicted", cleared)
self.assertIn("hazard:stale-lease", cleared)
self.assertIn("status:blocked", cleared)
def test_terminal_blocker_hazard_normalizes(self):
self.assertEqual(
labels.canonical_hazard_label("terminal-blocker"),
"hazard:terminal-blocker",
)
def test_assess_surfaces_hazard_labels(self):
result = labels.assess_issue_labels(
["type:bug", "status:blocked", "hazard:conflicted"]
)
self.assertEqual(result["hazard_labels"], ["hazard:conflicted"])
class TestDiscussionExclusion(unittest.TestCase):
def test_discussion_issue_excluded_from_implementation_queue(self):
self.assertTrue(labels.is_discussion(["type:discussion", "status:ready"]))
self.assertFalse(
labels.is_implementation_candidate(["type:discussion", "status:ready"])
)
def test_non_discussion_issue_is_candidate(self):
self.assertTrue(
labels.is_implementation_candidate(["type:feature", "status:ready"])
)
class TestBlockingReasonRequirement(unittest.TestCase):
def test_blocked_requires_reason(self):
self.assertTrue(
labels.requires_blocking_reason(["type:bug", "status:blocked"])
)
def test_hazard_requires_reason(self):
self.assertTrue(
labels.requires_blocking_reason(
["type:bug", "status:ready", "hazard:stale-lease"]
)
)
def test_clean_ready_issue_needs_no_reason(self):
self.assertFalse(
labels.requires_blocking_reason(["type:feature", "status:ready"])
)
class TestState603SynonymTransitions(unittest.TestCase):
def test_authoring_maps_to_in_progress(self):
self.assertEqual(
labels.canonical_status_label("authoring"), "status:in-progress"
)
def test_changes_requested_transition(self):
result = labels.transition_status_labels(
["type:feature", "status:needs-review"], "changes-requested"
)
self.assertEqual(result, ["type:feature", "status:changes-requested"])
def test_merge_ready_maps_to_approved(self):
self.assertEqual(labels.canonical_status_label("merge-ready"), "status:approved")
def test_abandoned_maps_to_wontfix(self):
self.assertEqual(labels.canonical_status_label("abandoned"), "status:wontfix")
def test_blocked_and_merged_transitions(self):
blocked = labels.transition_status_labels(
["type:bug", "status:in-progress"], "blocked"
)
self.assertEqual(blocked, ["type:bug", "status:blocked"])
merged = labels.transition_status_labels(
["type:feature", "status:approved"], "merged"
)
self.assertEqual(merged, ["type:feature", "status:reconcile"])
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+29 -34
View File
@@ -28,33 +28,31 @@ class TestLabelCreation(unittest.TestCase):
"""Verify create-or-skip logic for the label set.""" """Verify create-or-skip logic for the label set."""
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all")
@patch("manage_labels.api") @patch("manage_labels.api")
def test_skips_existing_labels(self, mock_api, _auth): def test_skips_existing_labels(self, mock_api, mock_get_all, _auth):
# Simulate all labels already exist # Simulate all labels already exist (paginated inventory #627)
existing = [_make_label(l["name"], i) for i, l in enumerate(manage_labels.LABELS)] existing = [_make_label(l["name"], i) for i, l in enumerate(manage_labels.LABELS)]
mock_api.return_value = existing # first call is GET /labels mock_get_all.return_value = existing
# Patch sys.argv to avoid --dry # Patch sys.argv to avoid --dry
with patch.object(sys, "argv", ["manage_labels.py"]): with patch.object(sys, "argv", ["manage_labels.py"]):
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
manage_labels.main() manage_labels.main()
# The GET call happens, but no POST calls for label creation mock_get_all.assert_called()
get_calls = [c for c in mock_api.call_args_list if c[0][0] == "GET"]
post_label_calls = [ post_label_calls = [
c for c in mock_api.call_args_list c for c in mock_api.call_args_list
if c[0][0] == "POST" and c[0][1] == "/labels" if c[0][0] == "POST" and c[0][1] == "/labels"
] ]
self.assertGreaterEqual(len(get_calls), 1)
self.assertEqual(len(post_label_calls), 0) self.assertEqual(len(post_label_calls), 0)
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_creates_missing_labels(self, mock_api, _auth): def test_creates_missing_labels(self, mock_api, _get_all, _auth):
# Simulate no existing labels # Simulate no existing labels
def side_effect(method, path, auth, payload=None): def side_effect(method, path, auth, payload=None):
if method == "GET" and "/labels" in path:
return [] # no existing labels
if method == "POST" and path == "/labels": if method == "POST" and path == "/labels":
return {"id": 999, "name": payload["name"]} return {"id": 999, "name": payload["name"]}
if method == "PUT": if method == "PUT":
@@ -79,15 +77,14 @@ class TestLabelCreation(unittest.TestCase):
class TestDryRun(unittest.TestCase): class TestDryRun(unittest.TestCase):
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_dry_run_makes_no_writes(self, mock_api, _auth): def test_dry_run_makes_no_writes(self, mock_api, _get_all, _auth):
mock_api.return_value = [] # no existing labels
with patch.object(sys, "argv", ["manage_labels.py", "--dry"]): with patch.object(sys, "argv", ["manage_labels.py", "--dry"]):
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
manage_labels.main() manage_labels.main()
# Only the GET call should be made, no POST or PUT # Dry run should not call write methods via api()
for c in mock_api.call_args_list: for c in mock_api.call_args_list:
method = c[0][0] method = c[0][0]
self.assertEqual(method, "GET", self.assertEqual(method, "GET",
@@ -100,13 +97,13 @@ class TestDryRun(unittest.TestCase):
class TestLabelMapping(unittest.TestCase): class TestLabelMapping(unittest.TestCase):
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all")
@patch("manage_labels.api") @patch("manage_labels.api")
def test_applies_mapping_to_issues(self, mock_api, _auth): def test_applies_mapping_to_issues(self, mock_api, mock_get_all, _auth):
existing = [_make_label(l["name"], i + 1) for i, l in enumerate(manage_labels.LABELS)] existing = [_make_label(l["name"], i + 1) for i, l in enumerate(manage_labels.LABELS)]
mock_get_all.return_value = existing
def side_effect(method, path, auth, payload=None): def side_effect(method, path, auth, payload=None):
if method == "GET":
return existing
if method == "PUT": if method == "PUT":
return [{"name": "applied"}] return [{"name": "applied"}]
return None return None
@@ -158,11 +155,10 @@ class TestModes(unittest.TestCase):
return [(c[0][0], c[0][1]) for c in mock_api.call_args_list] return [(c[0][0], c[0][1]) for c in mock_api.call_args_list]
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_create_labels_only_no_mapping(self, mock_api, _auth): def test_create_labels_only_no_mapping(self, mock_api, _get_all, _auth):
def se(method, path, auth, payload=None): def se(method, path, auth, payload=None):
if method == "GET":
return [] # no existing labels
if method == "POST" and path == "/labels": if method == "POST" and path == "/labels":
return {"id": 1, "name": payload["name"]} return {"id": 1, "name": payload["name"]}
return None return None
@@ -173,14 +169,14 @@ class TestModes(unittest.TestCase):
self.assertFalse(any(m[0] == "PUT" for m in methods)) # no mapping applied self.assertFalse(any(m[0] == "PUT" for m in methods)) # no mapping applied
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all")
@patch("manage_labels.api") @patch("manage_labels.api")
def test_apply_mapping_only_no_label_creation(self, mock_api, _auth): def test_apply_mapping_only_no_label_creation(self, mock_api, mock_get_all, _auth):
existing = [_make_label(l["name"], i + 1) existing = [_make_label(l["name"], i + 1)
for i, l in enumerate(manage_labels.LABELS)] for i, l in enumerate(manage_labels.LABELS)]
mock_get_all.return_value = existing
def se(method, path, auth, payload=None): def se(method, path, auth, payload=None):
if method == "GET":
return existing
if method == "PUT": if method == "PUT":
return [{"name": "applied"}] return [{"name": "applied"}]
return None return None
@@ -192,13 +188,10 @@ class TestModes(unittest.TestCase):
self.assertEqual(len(put_calls), len(manage_labels.MAPPING)) self.assertEqual(len(put_calls), len(manage_labels.MAPPING))
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[_make_label("chore", 5)])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_add_label_appends_to_issue(self, mock_api, _auth): def test_add_label_appends_to_issue(self, mock_api, _get_all, _auth):
existing = [_make_label("chore", 5)]
def se(method, path, auth, payload=None): def se(method, path, auth, payload=None):
if method == "GET":
return existing
if method == "POST": if method == "POST":
return [{"name": "chore"}] return [{"name": "chore"}]
return None return None
@@ -212,19 +205,21 @@ class TestModes(unittest.TestCase):
self.assertFalse(any(c[0][0] == "PUT" for c in mock_api.call_args_list)) self.assertFalse(any(c[0][0] == "PUT" for c in mock_api.call_args_list))
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_add_label_unknown_makes_no_write(self, mock_api, _auth): def test_add_label_unknown_makes_no_write(self, mock_api, _get_all, _auth):
mock_api.side_effect = lambda *a, **k: [] if a[0] == "GET" else None
manage_labels.main(["--add-label", "42", "ghost"]) manage_labels.main(["--add-label", "42", "ghost"])
# Only the GET label lookup; no POST/PUT for an undefined label. # No write via api() for an undefined label.
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)) self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)
or len(mock_api.call_args_list) == 0)
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api_get_all", return_value=[_make_label("chore", 5)])
@patch("manage_labels.api") @patch("manage_labels.api")
def test_add_label_dry_makes_no_write(self, mock_api, _auth): def test_add_label_dry_makes_no_write(self, mock_api, _get_all, _auth):
mock_api.side_effect = lambda *a, **k: [_make_label("chore", 5)] if a[0] == "GET" else None
manage_labels.main(["--dry", "--add-label", "42", "chore"]) manage_labels.main(["--dry", "--add-label", "42", "chore"])
self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)) self.assertTrue(all(c[0][0] == "GET" for c in mock_api.call_args_list)
or len(mock_api.call_args_list) == 0)
@patch("manage_labels.get_auth_header", return_value=FAKE_AUTH) @patch("manage_labels.get_auth_header", return_value=FAKE_AUTH)
@patch("manage_labels.api") @patch("manage_labels.api")
+3
View File
@@ -17,6 +17,7 @@ class TestMcpDaemonGuard(unittest.TestCase):
mcp_daemon_guard.ALLOW_DIRECT_IMPORT_ENV, mcp_daemon_guard.ALLOW_DIRECT_IMPORT_ENV,
"PYTEST_CURRENT_TEST", "PYTEST_CURRENT_TEST",
}} }}
env["GITEA_TEST_FORCE_UNSANCTIONED"] = "1"
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError): with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
mcp_daemon_guard.assert_sanctioned_mutation_runtime("test") mcp_daemon_guard.assert_sanctioned_mutation_runtime("test")
@@ -43,6 +44,7 @@ class TestMcpDaemonGuard(unittest.TestCase):
"PYTEST_CURRENT_TEST", "PYTEST_CURRENT_TEST",
} }
} }
env["GITEA_TEST_FORCE_UNSANCTIONED"] = "1"
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError): with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
mcp_daemon_guard.assert_keychain_access_allowed() mcp_daemon_guard.assert_keychain_access_allowed()
@@ -58,6 +60,7 @@ class TestMcpDaemonGuard(unittest.TestCase):
"PYTEST_CURRENT_TEST", "PYTEST_CURRENT_TEST",
} }
} }
env["GITEA_TEST_FORCE_UNSANCTIONED"] = "1"
with patch.dict(os.environ, env, clear=True): with patch.dict(os.environ, env, clear=True):
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError): with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
gitea_auth.get_auth_header("gitea.prgs.cc") gitea_auth.get_auth_header("gitea.prgs.cc")
+22 -3
View File
@@ -48,6 +48,22 @@ import gitea_config # noqa: E402
import mcp_server import mcp_server
import issue_lock_store import issue_lock_store
import gitea_auth
_orig_api_request = gitea_auth.api_request
def mockable_api_request(*args, **kwargs):
import mcp_server
return mcp_server.api_request(*args, **kwargs)
def setUpModule():
gitea_auth.api_request = mockable_api_request
patch("mcp_server._enforce_root_checkout_guard").start()
patch("mcp_server._enforce_branches_only_author_mutation").start()
def tearDownModule():
gitea_auth.api_request = _orig_api_request
patch.stopall()
FAKE_AUTH = "Basic dGVzdDp0ZXN0" FAKE_AUTH = "Basic dGVzdDp0ZXN0"
FULL_HEAD_SHA = "a" * 40 FULL_HEAD_SHA = "a" * 40
@@ -1364,11 +1380,11 @@ class TestReviewPR(unittest.TestCase):
self.assertIn("Review/Merge Blocked", result["message"]) self.assertIn("Review/Merge Blocked", result["message"])
self.assertIn("Author profile", result["message"]) self.assertIn("Author profile", result["message"])
@patch("mcp_server.api_get_all") @patch("mcp_server.api_fetch_page")
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
@patch("mcp_server.get_profile") @patch("mcp_server.get_profile")
def test_legacy_review_pr_self_approval_blocked(self, mock_get_profile, _auth, mock_api, mock_get_all): def test_legacy_review_pr_self_approval_blocked(self, mock_get_profile, _auth, mock_api, mock_fetch):
mock_get_profile.return_value = { mock_get_profile.return_value = {
"profile_name": "gitea-reviewer", "profile_name": "gitea-reviewer",
"allowed_operations": ["read", "approve"], "allowed_operations": ["read", "approve"],
@@ -1376,7 +1392,10 @@ class TestReviewPR(unittest.TestCase):
"base_url": None, "base_url": None,
} }
head_sha = FULL_HEAD_SHA head_sha = FULL_HEAD_SHA
mock_get_all.return_value = [{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}] mock_fetch.return_value = (
[{"number": 1, "title": "PR 1", "state": "open", "head": {"ref": "branch1", "sha": head_sha}, "base": {"ref": "master"}, "mergeable": True, "user": {"login": "jcwalker3"}}],
{"page": 1, "per_page": 50, "returned_count": 1, "has_more": False, "next_page": None, "is_final_page": True}
)
# mock_api responses: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility) # mock_api responses: 1) /user (inventory), 2) /user (eligibility), 3) /pulls/1 (eligibility)
mock_api.side_effect = [ mock_api.side_effect = [
{"login": "jcwalker3"}, # /api/v1/user (inventory) {"login": "jcwalker3"}, # /api/v1/user (inventory)
+8
View File
@@ -2,6 +2,7 @@
import os import os
import unittest import unittest
from unittest.mock import patch
import gitea_mcp_server as mcp_server import gitea_mcp_server as mcp_server
@@ -31,7 +32,14 @@ class TestPreflightReadSurvival(unittest.TestCase):
mcp_server._preflight_whoami_baseline_porcelain = None mcp_server._preflight_whoami_baseline_porcelain = None
mcp_server._preflight_capability_baseline_porcelain = None mcp_server._preflight_capability_baseline_porcelain = None
self.author_guard_patch = patch("gitea_mcp_server._enforce_branches_only_author_mutation")
self.author_guard_patch.start()
self.root_guard_patch = patch("gitea_mcp_server._enforce_root_checkout_guard")
self.root_guard_patch.start()
def tearDown(self): def tearDown(self):
self.root_guard_patch.stop()
self.author_guard_patch.stop()
mcp_server._preflight_whoami_called = self.orig_whoami mcp_server._preflight_whoami_called = self.orig_whoami
mcp_server._preflight_capability_called = self.orig_capability mcp_server._preflight_capability_called = self.orig_capability
mcp_server._preflight_whoami_violation = self.orig_whoami_violation mcp_server._preflight_whoami_violation = self.orig_whoami_violation
+15 -24
View File
@@ -64,54 +64,45 @@ class TestMarkIssueCLI(unittest.TestCase):
with self.assertRaises(SystemExit): with self.assertRaises(SystemExit):
mark_issue.main(["10", "bogus_action"]) mark_issue.main(["10", "bogus_action"])
@patch("mark_issue.api_get_all", return_value=[{"id": 101, "name": "status:in-progress"}])
@patch("mark_issue.api_request") @patch("mark_issue.api_request")
@patch("mark_issue.get_auth_header", return_value=FAKE_AUTH) @patch("mark_issue.get_auth_header", return_value=FAKE_AUTH)
def test_successful_start(self, _auth, mock_api): def test_successful_start(self, _auth, mock_api, mock_get_all):
# First call is GET labels, second is POST label mock_api.return_value = [{"name": "status:in-progress"}]
mock_api.side_effect = [
[{"id": 101, "name": "status:in-progress"}],
[{"name": "status:in-progress"}],
]
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
rc = mark_issue.main(["15", "start"]) rc = mark_issue.main(["15", "start"])
self.assertEqual(rc, 0) self.assertEqual(rc, 0)
self.assertEqual(mock_api.call_count, 2) mock_get_all.assert_called()
self.assertIn("/labels", mock_get_all.call_args[0][0])
# Verify GET labels call
get_call = mock_api.call_args_list[0]
self.assertEqual(get_call[0][0], "GET")
self.assertIn("/labels?limit=100", get_call[0][1])
# Verify POST labels call # Verify POST labels call
post_call = mock_api.call_args_list[1] post_call = mock_api.call_args_list[0]
self.assertEqual(post_call[0][0], "POST") self.assertEqual(post_call[0][0], "POST")
self.assertIn("/issues/15/labels", post_call[0][1]) self.assertIn("/issues/15/labels", post_call[0][1])
self.assertEqual(post_call[0][3], {"labels": [101]}) self.assertEqual(post_call[0][3], {"labels": [101]})
@patch("mark_issue.api_get_all", return_value=[{"id": 101, "name": "status:in-progress"}])
@patch("mark_issue.api_request") @patch("mark_issue.api_request")
@patch("mark_issue.get_auth_header", return_value=FAKE_AUTH) @patch("mark_issue.get_auth_header", return_value=FAKE_AUTH)
def test_successful_done(self, _auth, mock_api): def test_successful_done(self, _auth, mock_api, _get_all):
# First call is GET labels, second is DELETE label mock_api.return_value = None
mock_api.side_effect = [
[{"id": 101, "name": "status:in-progress"}],
None,
]
rc = mark_issue.main(["15", "done"]) rc = mark_issue.main(["15", "done"])
self.assertEqual(rc, 0) self.assertEqual(rc, 0)
self.assertEqual(mock_api.call_count, 2) self.assertEqual(mock_api.call_count, 1)
# Verify DELETE labels call # Verify DELETE labels call
delete_call = mock_api.call_args_list[1] delete_call = mock_api.call_args_list[0]
self.assertEqual(delete_call[0][0], "DELETE") self.assertEqual(delete_call[0][0], "DELETE")
self.assertIn("/issues/15/labels/101", delete_call[0][1]) self.assertIn("/issues/15/labels/101", delete_call[0][1])
@patch("mark_issue.api_get_all", return_value=[{"id": 1, "name": "bug"}])
@patch("mark_issue.api_request") @patch("mark_issue.api_request")
@patch("mark_issue.get_auth_header", return_value=FAKE_AUTH) @patch("mark_issue.get_auth_header", return_value=FAKE_AUTH)
def test_label_not_found(self, _auth, mock_api): def test_label_not_found(self, _auth, mock_api, _get_all):
# GET labels returns no status:in-progress label # Paginated inventory returns no status:in-progress label
mock_api.return_value = [{"id": 1, "name": "bug"}]
rc = mark_issue.main(["15", "start"]) rc = mark_issue.main(["15", "start"])
self.assertEqual(rc, 1) self.assertEqual(rc, 1)
mock_api.assert_not_called()
if __name__ == "__main__": if __name__ == "__main__":
@@ -10,8 +10,11 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import gitea_mcp_server as srv import gitea_mcp_server as srv
FAKE_AUTH = "token test" FAKE_AUTH = "token test"
CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
if "branches" in current_file_path.parts:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[3])
else:
CONTROL_CHECKOUT_ROOT = str(current_file_path.parents[1])
RECONCILER_PROFILE = { RECONCILER_PROFILE = {
"profile_name": "prgs-reconciler", "profile_name": "prgs-reconciler",
"allowed_operations": ["gitea.read", "gitea.pr.close", "gitea.pr.comment"], "allowed_operations": ["gitea.read", "gitea.pr.close", "gitea.pr.comment"],
+16 -4
View File
@@ -13,8 +13,13 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import gitea_mcp_server as srv # noqa: E402 import gitea_mcp_server as srv # noqa: E402
import root_checkout_guard as rcg # noqa: E402 import root_checkout_guard as rcg # noqa: E402
CONTROL_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
BRANCHES_WORKTREE = str(Path(__file__).resolve().parents[1]) if "branches" in current_file_path.parts:
CONTROL_ROOT = str(current_file_path.parents[3])
BRANCHES_WORKTREE = str(current_file_path.parents[1])
else:
CONTROL_ROOT = str(current_file_path.parents[1])
BRANCHES_WORKTREE = str(current_file_path.parents[1] / "branches" / "mock-worktree")
MASTER_SHA = "a" * 40 MASTER_SHA = "a" * 40
OTHER_SHA = "b" * 40 OTHER_SHA = "b" * 40
@@ -136,13 +141,21 @@ class TestVerifyPreflightRootGuardIntegration(unittest.TestCase):
self.assertIn("Root checkout guard (#475)", str(ctx.exception)) self.assertIn("Root checkout guard (#475)", str(ctx.exception))
self.assertIn(rcg.REMEDIATION, str(ctx.exception)) self.assertIn(rcg.REMEDIATION, str(ctx.exception))
@patch("os.path.isdir", return_value=True)
@patch("os.path.exists", return_value=True)
@patch("subprocess.run")
@patch("gitea_mcp_server._get_workspace_porcelain", return_value="") @patch("gitea_mcp_server._get_workspace_porcelain", return_value="")
@patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value=MASTER_SHA) @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value=MASTER_SHA)
@patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state") @patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state")
@patch("gitea_mcp_server._resolve_author_mutation_context") @patch("gitea_mcp_server._resolve_author_mutation_context")
def test_reviewer_from_branches_worktree_allowed( def test_reviewer_from_branches_worktree_allowed(
self, mock_ctx, mock_git, _remote_sha, _porcelain, self, mock_ctx, mock_git, _remote_sha, _porcelain, mock_run, _exists, _isdir,
): ):
import unittest.mock
mock_run.return_value = unittest.mock.MagicMock(
returncode=0,
stdout=f"{CONTROL_ROOT}/.git\n",
)
srv._preflight_capability_baseline_porcelain = "" srv._preflight_capability_baseline_porcelain = ""
mock_ctx.return_value = { mock_ctx.return_value = {
"workspace_path": BRANCHES_WORKTREE, "workspace_path": BRANCHES_WORKTREE,
@@ -156,6 +169,5 @@ class TestVerifyPreflightRootGuardIntegration(unittest.TestCase):
} }
srv.verify_preflight_purity("prgs", worktree_path=BRANCHES_WORKTREE) srv.verify_preflight_purity("prgs", worktree_path=BRANCHES_WORKTREE)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+318
View File
@@ -0,0 +1,318 @@
"""#627: paginated repository-label resolution for gitea_set_issue_labels.
Reproduces the post-merge #601 reconciliation failure where later-page
labels (e.g. type:feature, workflow-hardening) were falsely rejected as
nonexistent because inventory used a single-page GET labels?limit=100.
"""
from __future__ import annotations
import os
import sys
import unittest
from unittest.mock import patch, call
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import mcp_server
import gitea_auth
FAKE_AUTH = "token test-token"
PAGE_SIZE = 50 # Gitea effective max; see gitea_auth.api_get_all
def _lb(name: str, lid: int) -> dict:
return {"id": lid, "name": name, "color": "000000"}
def _pages(labels: list[dict], page_size: int = PAGE_SIZE) -> list[list[dict]]:
if not labels:
return [[]]
pages = []
for i in range(0, len(labels), page_size):
pages.append(labels[i : i + page_size])
return pages
class TestRepoLabelIdMapPagination(unittest.TestCase):
"""_repo_label_id_map must use api_get_all (all pages)."""
@patch("mcp_server.api_get_all")
def test_fewer_than_one_page(self, mock_all):
labels = [_lb(f"l{i}", i) for i in range(3)]
mock_all.return_value = labels
m = mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertEqual(m, {"l0": 0, "l1": 1, "l2": 2})
mock_all.assert_called_once()
self.assertIn("/labels", mock_all.call_args[0][0])
self.assertNotIn("limit=100", mock_all.call_args[0][0])
@patch("mcp_server.api_get_all")
def test_exactly_one_full_page(self, mock_all):
labels = [_lb(f"l{i:03d}", i) for i in range(PAGE_SIZE)]
mock_all.return_value = labels
m = mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertEqual(len(m), PAGE_SIZE)
self.assertEqual(m["l000"], 0)
self.assertEqual(m[f"l{PAGE_SIZE - 1:03d}"], PAGE_SIZE - 1)
@patch("mcp_server.api_get_all")
def test_more_than_one_page_includes_later_labels(self, mock_all):
# Page 1: 50 early labels; page 2: type:feature + workflow-hardening (#601 style)
early = [_lb(f"early-{i:02d}", i) for i in range(PAGE_SIZE)]
late = [
_lb("type:feature", 1001),
_lb("workflow-hardening", 1002),
]
mock_all.return_value = early + late
m = mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertEqual(len(m), PAGE_SIZE + 2)
self.assertEqual(m["type:feature"], 1001)
self.assertEqual(m["workflow-hardening"], 1002)
@patch("mcp_server.api_get_all")
def test_duplicate_names_keep_first_seen_id(self, mock_all):
mock_all.return_value = [
_lb("type:feature", 103),
_lb("type:feature", 130), # duplicate id later
_lb("workflow-hardening", 105),
_lb("workflow-hardening", 128),
]
m = mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertEqual(m["type:feature"], 103)
self.assertEqual(m["workflow-hardening"], 105)
@patch("mcp_server.api_get_all", return_value={"not": "a list"})
def test_non_list_inventory_fails_closed(self, _all):
with self.assertRaises(RuntimeError) as ctx:
mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertIn("expected a list", str(ctx.exception).lower())
@patch("mcp_server.api_get_all", side_effect=RuntimeError("page 2 failed: connection reset"))
def test_later_page_failure_propagates(self, _all):
with self.assertRaises(RuntimeError) as ctx:
mcp_server._repo_label_id_map("https://gitea.example/api/v1/repos/o/r", FAKE_AUTH)
self.assertIn("page 2 failed", str(ctx.exception))
class TestSetIssueLabelsPagination(unittest.TestCase):
"""gitea_set_issue_labels full-set replacement with multi-page inventory."""
def setUp(self):
self._remotes = patch.dict(
mcp_server.REMOTES,
{"prgs": {"host": "gitea.example.com", "org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools"}},
)
self._remotes.start()
# Allow mutation path without full preflight stack when possible
self._preflight = patch(
"mcp_server.verify_preflight_purity", return_value=None
)
self._preflight.start()
self._perm = patch(
"mcp_server._profile_permission_block", return_value=None
)
self._perm.start()
self._auth = patch(
"mcp_server._auth", return_value=FAKE_AUTH
)
self._auth.start()
self._audited = patch("mcp_server._audited")
mock_aud = self._audited.start()
mock_aud.return_value.__enter__ = lambda s: None
mock_aud.return_value.__exit__ = lambda s, *a: None
def tearDown(self):
self._remotes.stop()
self._preflight.stop()
self._perm.stop()
self._auth.stop()
self._audited.stop()
def _inventory_early_and_late(self) -> list[dict]:
early = [_lb(f"alpha-{i:02d}", i + 1) for i in range(PAGE_SIZE)]
late = [
_lb("type:feature", 9001),
_lb("workflow-hardening", 9002),
_lb("status:ready", 9003),
_lb("anti-stomp", 9004),
_lb("leases", 9005),
_lb("recovery", 9006),
]
return early + late
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_requested_labels_split_across_pages(self, mock_all, mock_req):
inv = self._inventory_early_and_late()
mock_all.return_value = inv
requested = ["alpha-00", "type:feature", "workflow-hardening"]
mock_req.return_value = [_lb(n, inv_i["id"]) for n in requested
for inv_i in inv if inv_i["name"] == n]
res = mcp_server.gitea_set_issue_labels(
issue_number=601,
labels=requested,
remote="prgs",
)
self.assertEqual({lb["name"] for lb in res}, set(requested))
# PUT must use ids from both pages
put = mock_req.call_args
self.assertEqual(put[0][0], "PUT")
payload_ids = put[0][3]["labels"]
self.assertEqual(payload_ids, [1, 9001, 9002])
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_missing_requested_label_rejected_before_put(self, mock_all, mock_req):
mock_all.return_value = [_lb("bug", 1), _lb("status:ready", 2)]
with self.assertRaises(RuntimeError) as ctx:
mcp_server.gitea_set_issue_labels(
issue_number=9,
labels=["bug", "does-not-exist"],
remote="prgs",
)
self.assertIn("do not exist", str(ctx.exception))
self.assertIn("does-not-exist", str(ctx.exception))
mock_req.assert_not_called()
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_complete_set_preserves_later_page_labels(self, mock_all, mock_req):
inv = self._inventory_early_and_late()
mock_all.return_value = inv
# Full-set replacement removing only status:pr-open style stale label:
# keep later-page feature labels (the #601 reconciliation shape).
requested = [
"anti-stomp",
"leases",
"recovery",
"type:feature",
"workflow-hardening",
]
mock_req.return_value = [_lb(n, next(x["id"] for x in inv if x["name"] == n))
for n in requested]
res = mcp_server.gitea_set_issue_labels(
issue_number=601, labels=requested, remote="prgs"
)
self.assertEqual([lb["name"] for lb in res], requested)
payload_ids = mock_req.call_args[0][3]["labels"]
self.assertEqual(payload_ids, [9004, 9005, 9006, 9001, 9002])
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_issue_601_regression_later_page_names_accepted(self, mock_all, mock_req):
"""Regression: #601 reconciler rejected type:feature + workflow-hardening.
Single-page inventory would omit them; paginated inventory must accept.
"""
early = [_lb(f"page1-{i:02d}", i + 1) for i in range(PAGE_SIZE)]
# Exactly the labels from the #601 residual set (minus status:pr-open)
late = [
_lb("type:feature", 103),
_lb("workflow-hardening", 105),
_lb("anti-stomp", 106),
_lb("leases", 109),
_lb("recovery", 110),
]
mock_all.return_value = early + late
requested = [
"anti-stomp",
"leases",
"recovery",
"type:feature",
"workflow-hardening",
]
mock_req.return_value = [
_lb(n, next(x["id"] for x in late if x["name"] == n)) for n in requested
]
res = mcp_server.gitea_set_issue_labels(
issue_number=601, labels=requested, remote="prgs"
)
names = {lb["name"] for lb in res}
self.assertEqual(names, set(requested))
self.assertNotIn("status:pr-open", names)
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_duplicate_label_ids_resolve_deterministically(self, mock_all, mock_req):
mock_all.return_value = [
_lb("type:feature", 103),
_lb("type:feature", 130),
_lb("workflow-hardening", 105),
_lb("workflow-hardening", 128),
]
requested = ["type:feature", "workflow-hardening"]
mock_req.return_value = [_lb("type:feature", 103), _lb("workflow-hardening", 105)]
mcp_server.gitea_set_issue_labels(
issue_number=1, labels=requested, remote="prgs"
)
self.assertEqual(mock_req.call_args[0][3]["labels"], [103, 105])
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_post_mutation_mismatch_fails_closed(self, mock_all, mock_req):
mock_all.return_value = [_lb("bug", 1), _lb("status:ready", 2)]
# Server returns incomplete set → verification must fail
mock_req.return_value = [_lb("bug", 1)]
with self.assertRaises(RuntimeError) as ctx:
mcp_server.gitea_set_issue_labels(
issue_number=9,
labels=["bug", "status:ready"],
remote="prgs",
)
self.assertIn("Post-mutation label verification failed", str(ctx.exception))
self.assertIn("status:ready", str(ctx.exception))
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all", side_effect=RuntimeError("Gitea 502 on page 2"))
def test_pagination_failure_fails_closed_before_put(self, _all, mock_req):
with self.assertRaises(RuntimeError) as ctx:
mcp_server.gitea_set_issue_labels(
issue_number=9, labels=["bug"], remote="prgs"
)
self.assertIn("page 2", str(ctx.exception))
mock_req.assert_not_called()
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_empty_label_set_clears_all(self, mock_all, mock_req):
mock_all.return_value = [_lb("bug", 1)]
mock_req.return_value = []
res = mcp_server.gitea_set_issue_labels(
issue_number=9, labels=[], remote="prgs"
)
self.assertEqual(res, [])
self.assertEqual(mock_req.call_args[0][3]["labels"], [])
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
def test_does_not_call_single_page_limit_100(self, mock_all, mock_req):
mock_all.return_value = [_lb("bug", 1)]
mock_req.return_value = [_lb("bug", 1)]
mcp_server.gitea_set_issue_labels(
issue_number=9, labels=["bug"], remote="prgs"
)
for c in mock_req.call_args_list:
url = c[0][1] if len(c[0]) > 1 else ""
self.assertNotIn("labels?limit=100", str(url))
mock_all.assert_called()
class TestApiGetAllPageCapDocumentsGiteaLimit(unittest.TestCase):
"""Sanity: api_get_all clamps page_size to 50 (root cause of limit=100 trap)."""
@patch("gitea_auth.api_request")
def test_page_size_clamped_to_fifty(self, mock_req):
mock_req.return_value = []
gitea_auth.api_get_all("https://gitea.example/api/v1/repos/o/r/labels",
FAKE_AUTH, page_size=100)
# First (only) call must use limit=50, not 100
url = mock_req.call_args[0][1]
self.assertIn("limit=50", url)
self.assertNotIn("limit=100", url)
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,188 @@
"""Server wiring for stable-branch push contamination (#671).
Exercises the MCP tools and the pre-flight enforcement gate against the durable
contamination marker (isolated per-test state dir from conftest).
"""
from __future__ import annotations
import os
from unittest.mock import patch
import gitea_mcp_server as srv
def _clear_marker(remote="prgs"):
srv._clear_stable_contamination_marker(remote=remote)
def teardown_function():
_clear_marker()
# ── record tool marks a real direct push ─────────────────────────────────────
def test_record_tool_marks_direct_master_push():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
assert res["contaminated"] is True
assert res["marked"] is True
assert res["marker"] is not None
assert res["marker"]["reason_class"] == "stable_branch_push"
# loaded back through the durable store
loaded = srv._load_stable_contamination_marker("prgs")
assert loaded is not None
assert loaded["ref"] == "master"
def test_record_tool_dry_run_still_marks():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git push --dry-run prgs master", remote="prgs"
)
assert res["contaminated"] is True
assert res["marked"] is True
def test_record_tool_feature_branch_does_not_mark():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git push prgs fix/issue-671-block-stable-branch-push",
remote="prgs",
)
assert res["contaminated"] is False
assert res["marked"] is False
assert srv._load_stable_contamination_marker("prgs") is None
def test_record_tool_fetch_only_does_not_mark():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git fetch prgs", remote="prgs"
)
assert res["contaminated"] is False
assert res["marked"] is False
def test_record_tool_mark_false_is_readonly():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs", mark=False
)
assert res["contaminated"] is True
assert res["marked"] is False
assert srv._load_stable_contamination_marker("prgs") is None
def test_record_tool_redacts_secret_in_marker():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command="git push https://u:supersecret@host/o/r.git master",
remote="prgs",
)
assert res["marked"] is True
assert "supersecret" not in res["marker"]["command_summary"]
def test_record_tool_root_checkout_local_commit_marks():
_clear_marker()
res = srv.gitea_record_stable_branch_push_attempt(
command=None,
remote="prgs",
current_branch="master",
head_sha="a" * 40,
remote_master_sha="b" * 40,
is_under_branches=False,
)
assert res["contaminated"] is True
assert res["marked"] is True
assert res["marker"]["reason_class"] == "root_checkout_commit"
# ── audit tool: inspect + reconciler-only clear ──────────────────────────────
def test_audit_inspect_reports_marker():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
out = srv.gitea_audit_stable_branch_contamination(action="inspect", remote="prgs")
assert out["contaminated"] is True
assert out["read_only"] is True
def test_audit_clear_refused_for_non_reconciler():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
with patch.object(srv, "_actual_profile_role", return_value="author"):
out = srv.gitea_audit_stable_branch_contamination(action="clear", remote="prgs")
assert out["success"] is False
assert out["reasons"]
# marker survives a refused clear
assert srv._load_stable_contamination_marker("prgs") is not None
def test_audit_clear_allowed_for_reconciler():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
identity = srv._stable_contamination_profile_identity()
with patch.object(srv, "_actual_profile_role", return_value="reconciler"):
out = srv.gitea_audit_stable_branch_contamination(
action="clear", remote="prgs", profile_identity=identity
)
assert out["success"] is True
assert srv._load_stable_contamination_marker("prgs") is None
# ── pre-flight enforcement gate ──────────────────────────────────────────────
def _force_gate_env():
return patch.dict(os.environ, {"GITEA_TEST_FORCE_STABLE_CONTAMINATION": "1"})
def test_gate_blocks_gated_mutation_when_contaminated():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
with _force_gate_env(), patch.object(srv, "_actual_profile_role", return_value="author"):
for task in ("merge_pr", "review_pr", "close_issue", "create_pr"):
try:
srv._enforce_stable_branch_contamination_gate(task, "prgs")
raised = False
except RuntimeError as exc:
raised = True
assert "#671" in str(exc)
assert raised, task
def test_gate_allows_comment_for_handoff_when_contaminated():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
with _force_gate_env(), patch.object(srv, "_actual_profile_role", return_value="author"):
# must not raise — worker can still post the durable audit comment
srv._enforce_stable_branch_contamination_gate("comment_issue", "prgs")
srv._enforce_stable_branch_contamination_gate("lock_issue", "prgs")
def test_gate_exempts_reconciler_when_contaminated():
_clear_marker()
srv.gitea_record_stable_branch_push_attempt(
command="git push prgs master", remote="prgs"
)
with _force_gate_env(), patch.object(srv, "_actual_profile_role", return_value="reconciler"):
srv._enforce_stable_branch_contamination_gate("merge_pr", "prgs")
def test_gate_noop_when_not_contaminated():
_clear_marker()
with _force_gate_env(), patch.object(srv, "_actual_profile_role", return_value="author"):
srv._enforce_stable_branch_contamination_gate("merge_pr", "prgs")
+341
View File
@@ -0,0 +1,341 @@
"""Tests for the direct stable-branch push guard (#671).
Covers the acceptance criteria:
1. detect shell ``git push <remote> master`` equivalents
2. detect root/control-checkout local commits not on an issue branch
3. contamination record shape
4. fail-closed gate on review/merge/close/completion (reconciler-exempt)
5. AC5 case matrix: no-op dry-run push, real direct push, sanctioned Gitea
merge, fetch-only, root-checkout local commit; plus feature-branch push
still allowed and sanctioned merge still allowed
6. redaction of credentials in logged command summaries
"""
import stable_branch_push_guard as guard
# ── AC1: detect direct stable-branch push equivalents ────────────────────────
def test_plain_git_push_remote_master_is_contamination():
res = guard.classify_push_command("git push prgs master")
assert res["is_git_push"] is True
assert res["targets_stable"] is True
assert res["stable_refs"] == ["master"]
assert res["contamination"] is True
assert res["reasons"]
def test_push_main_and_dev_detected():
for ref in ("main", "dev", "develop", "development"):
res = guard.classify_push_command(f"git push origin {ref}")
assert res["contamination"] is True, ref
assert res["stable_refs"] == [ref]
def test_head_colon_master_refspec_detected():
res = guard.classify_push_command("git push prgs HEAD:master")
assert res["targets_stable"] is True
assert res["stable_refs"] == ["master"]
assert res["contamination"] is True
def test_full_refspec_force_plus_detected():
res = guard.classify_push_command(
"git push prgs +refs/heads/tmp:refs/heads/master"
)
assert res["contamination"] is True
assert res["is_force"] is True
assert res["stable_refs"] == ["master"]
def test_force_flag_to_master_detected():
res = guard.classify_push_command("git push --force prgs master")
assert res["contamination"] is True
assert res["is_force"] is True
def test_delete_refspec_master_detected():
res = guard.classify_push_command("git push prgs :master")
assert res["contamination"] is True
assert res["is_delete"] is True
def test_delete_flag_master_detected():
res = guard.classify_push_command("git push --delete prgs master")
assert res["contamination"] is True
assert res["is_delete"] is True
def test_push_detected_inside_compound_command():
res = guard.classify_push_command("cd repo && git push prgs master && echo ok")
assert res["contamination"] is True
assert res["stable_refs"] == ["master"]
# ── AC5: no-op / dry-run push still proves intent ────────────────────────────
def test_dry_run_push_to_master_proves_intent():
res = guard.classify_push_command("git push --dry-run prgs master")
assert res["is_dry_run"] is True
assert res["contamination"] is True
assert res["proves_intent"] is True
assert "intent" in " ".join(res["reasons"]).lower()
def test_short_dry_run_flag_n_detected():
res = guard.classify_push_command("git push -n prgs master")
assert res["is_dry_run"] is True
assert res["contamination"] is True
# ── AC5 negative: feature-branch push still allowed ──────────────────────────
def test_feature_branch_push_not_flagged():
res = guard.classify_push_command(
"git push prgs fix/issue-671-block-stable-branch-push"
)
assert res["is_git_push"] is True
assert res["targets_stable"] is False
assert res["contamination"] is False
assert res["reasons"] == []
def test_feature_branch_head_refspec_not_flagged():
res = guard.classify_push_command(
"git push prgs HEAD:fix/issue-671-block-stable-branch-push"
)
assert res["contamination"] is False
assert res["targets_stable"] is False
def test_branch_named_like_master_substring_not_flagged():
# 'master-notes' is not the stable 'master'.
res = guard.classify_push_command("git push prgs master-notes")
assert res["contamination"] is False
assert res["targets_stable"] is False
# ── AC5 negative: fetch-only operations ──────────────────────────────────────
def test_git_fetch_not_a_push():
res = guard.classify_push_command("git fetch prgs")
assert res["is_git_push"] is False
assert res["is_fetch_or_pull"] is True
assert res["contamination"] is False
def test_git_pull_ff_only_master_not_a_push():
res = guard.classify_push_command("git pull --ff-only prgs master")
assert res["is_git_push"] is False
assert res["is_fetch_or_pull"] is True
assert res["contamination"] is False
# ── AC5 negative: sanctioned Gitea merge is not a push ───────────────────────
def test_gitea_merge_pr_tool_is_not_a_push():
res = guard.classify_push_command("gitea_merge_pr(pr_number=671, remote='prgs')")
assert res["is_git_push"] is False
assert res["contamination"] is False
def test_gitea_api_merge_is_not_a_push():
res = guard.classify_push_command(
"curl -X POST https://gitea.example/api/v1/repos/o/r/pulls/671/merge"
)
assert res["is_git_push"] is False
assert res["contamination"] is False
# ── ambiguous bare push: reported, not auto-contaminating ────────────────────
def test_bare_push_is_ambiguous_not_contaminating():
res = guard.classify_push_command("git push prgs")
assert res["is_git_push"] is True
assert res["ambiguous_target"] is True
assert res["contamination"] is False
assert res["reasons"] # surfaced as a warning
# ── AC2: root/control-checkout local commit detection ────────────────────────
def test_root_checkout_commit_ahead_of_master_flagged():
res = guard.assess_root_checkout_local_commit(
current_branch="master",
head_sha="a" * 40,
remote_master_sha="b" * 40,
is_under_branches=False,
)
assert res["contamination"] is True
assert res["reasons"]
def test_root_checkout_clean_at_master_not_flagged():
sha = "c" * 40
res = guard.assess_root_checkout_local_commit(
current_branch="master",
head_sha=sha,
remote_master_sha=sha,
is_under_branches=False,
)
assert res["contamination"] is False
assert res["unknown"] is False
def test_branches_worktree_commit_is_exempt():
res = guard.assess_root_checkout_local_commit(
current_branch="fix/issue-671-block-stable-branch-push",
head_sha="a" * 40,
remote_master_sha="b" * 40,
is_under_branches=True,
)
assert res["contamination"] is False
def test_root_checkout_ahead_count_flagged():
res = guard.assess_root_checkout_local_commit(
current_branch="master",
head_sha="",
remote_master_sha="",
is_under_branches=False,
ahead_count=2,
)
assert res["contamination"] is True
def test_root_checkout_unknown_when_state_missing():
res = guard.assess_root_checkout_local_commit(
current_branch="master",
head_sha="",
remote_master_sha="",
is_under_branches=False,
)
assert res["contamination"] is False
assert res["unknown"] is True
def test_root_checkout_non_stable_branch_out_of_scope():
res = guard.assess_root_checkout_local_commit(
current_branch="feature/x",
head_sha="a" * 40,
remote_master_sha="b" * 40,
is_under_branches=False,
)
assert res["contamination"] is False
# ── AC3: contamination record shape ──────────────────────────────────────────
def test_build_contamination_record_shape_and_redaction():
rec = guard.build_contamination_record(
reason_class="stable_branch_push",
command_redacted="git push https://user:[email protected]/o/r.git master",
session_id="prgs-author-123",
remote="prgs",
ref="master",
role="author",
)
assert rec["kind"] == guard.CONTAMINATION_KIND
assert rec["reason_class"] == "stable_branch_push"
assert rec["remote"] == "prgs"
assert rec["ref"] == "master"
assert rec["cleared_by_reconciler"] is False
# secret must never survive into the durable record
assert "tok@" not in rec["command_summary"]
assert "***@" in rec["command_summary"]
# ── AC4: fail-closed gate on gated mutations ─────────────────────────────────
def _marker():
return guard.build_contamination_record(
reason_class="stable_branch_push",
command_redacted="git push prgs master",
remote="prgs",
ref="master",
role="author",
)
def test_gate_blocks_gated_mutations_for_author():
marker = _marker()
for task in ("create_pr", "merge_pr", "close_issue", "review_pr",
"submit_pr_review", "commit_files", "close_pr"):
gate = guard.assess_contamination_gate(marker, task=task, actual_role="author")
assert gate["block"] is True, task
assert gate["reasons"]
def test_gate_allows_comment_and_lock_for_handoff():
marker = _marker()
for task in ("comment_issue", "lock_issue", "create_issue", "mark_issue"):
gate = guard.assess_contamination_gate(marker, task=task, actual_role="author")
assert gate["block"] is False, task
def test_gate_exempts_reconciler_audit_path():
marker = _marker()
gate = guard.assess_contamination_gate(marker, task="close_pr", actual_role="reconciler")
assert gate["block"] is False
def test_gate_no_marker_allows_everything():
gate = guard.assess_contamination_gate(None, task="merge_pr", actual_role="merger")
assert gate["block"] is False
def test_gate_cleared_marker_allows_everything():
marker = _marker()
marker["cleared_by_reconciler"] = True
gate = guard.assess_contamination_gate(marker, task="merge_pr", actual_role="merger")
assert gate["block"] is False
def test_same_worker_cannot_self_clear_by_role():
# A merger/reviewer/author role is still gated — only reconciler is exempt.
marker = _marker()
for role in ("author", "reviewer", "merger"):
gate = guard.assess_contamination_gate(marker, task="merge_pr", actual_role=role)
assert gate["block"] is True, role
def test_format_gate_error_mentions_issue():
marker = _marker()
gate = guard.assess_contamination_gate(marker, task="merge_pr", actual_role="merger")
msg = guard.format_contamination_gate_error(gate)
assert "#671" in msg
assert "contaminat" in msg.lower()
# ── redaction unit coverage ──────────────────────────────────────────────────
def test_redact_url_userinfo():
out = guard.redact_command("git push https://bob:secretpat@host/o/r.git master")
assert "secretpat" not in out
assert "***@" in out
assert "master" in out # structure preserved for audit
def test_redact_token_assignment():
out = guard.redact_command("GITEA_TOKEN=abcdef123456 git push prgs master")
assert "abcdef123456" not in out
assert "GITEA_TOKEN=***" in out
def test_redact_empty():
assert guard.redact_command(None) == ""
assert guard.redact_command("") == ""
# ── detect_stable_push over iterables ────────────────────────────────────────
def test_detect_stable_push_iterable_finds_first_contamination():
cmds = ["git status", "git push prgs master", "echo done"]
res = guard.detect_stable_push(cmds)
assert res["contamination"] is True
def test_detect_stable_push_iterable_all_safe():
cmds = ["git status", "git push prgs feature/x", "git fetch prgs"]
res = guard.detect_stable_push(cmds)
assert res["contamination"] is False
+24 -3
View File
@@ -14,8 +14,13 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import author_mutation_worktree as amw # noqa: E402 import author_mutation_worktree as amw # noqa: E402
import gitea_mcp_server as srv # noqa: E402 import gitea_mcp_server as srv # noqa: E402
CONTROL_ROOT = str(Path(__file__).resolve().parents[3]) current_file_path = Path(__file__).resolve()
BRANCHES_WORKTREE = str(Path(__file__).resolve().parents[1]) if "branches" in current_file_path.parts:
CONTROL_ROOT = str(current_file_path.parents[3])
BRANCHES_WORKTREE = str(current_file_path.parents[1])
else:
CONTROL_ROOT = str(current_file_path.parents[1])
BRANCHES_WORKTREE = str(current_file_path.parents[1] / "branches" / "mock-worktree")
MCP_PROCESS_ROOT = BRANCHES_WORKTREE MCP_PROCESS_ROOT = BRANCHES_WORKTREE
@@ -95,7 +100,23 @@ class TestRuntimeContextGuardAlignment(unittest.TestCase):
srv._preflight_in_test_mode = self._orig_in_test srv._preflight_in_test_mode = self._orig_in_test
self._env_patch.stop() self._env_patch.stop()
def test_runtime_context_and_guard_share_resolved_workspace(self): @mock.patch("subprocess.run")
@mock.patch("os.path.isdir", return_value=True)
@mock.patch("os.path.exists", return_value=True)
def test_runtime_context_and_guard_share_resolved_workspace(
self, _exists, _isdir, mock_run
):
def run_side_effect(cmd, *args, **kwargs):
res = MagicMock(returncode=0)
if "--git-common-dir" in cmd:
res.stdout = f"{CONTROL_ROOT}/.git\n"
elif "--show-toplevel" in cmd:
cwd = cmd[cmd.index("-C") + 1] if "-C" in cmd else ""
res.stdout = f"{cwd}\n"
else:
res.stdout = f"{CONTROL_ROOT}\n"
return res
mock_run.side_effect = run_side_effect
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT): with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
ctx = srv._resolve_author_mutation_context(BRANCHES_WORKTREE) ctx = srv._resolve_author_mutation_context(BRANCHES_WORKTREE)
status = srv.assess_preflight_status(worktree_path=BRANCHES_WORKTREE) status = srv.assess_preflight_status(worktree_path=BRANCHES_WORKTREE)
+1 -5
View File
@@ -14,11 +14,7 @@ from merged_cleanup_reconcile import (
read_issue_lock, read_issue_lock,
read_local_worktree_state, read_local_worktree_state,
) )
from reviewer_worktree import REVIEW_WORKTREE_RE
_REVIEW_WORKTREE_RE = re.compile(
r"branches/(?:review-pr\d+|merge-simulation-pr\d+|review-[\w-]+)",
re.IGNORECASE,
)
CLASSIFICATIONS = frozenset({ CLASSIFICATIONS = frozenset({
"active-pr", "active-pr",
+3 -5
View File
@@ -35,7 +35,7 @@ from datetime import datetime, timezone
from typing import Any from typing import Any
from merged_cleanup_reconcile import branch_worktree_folder, read_local_worktree_state from merged_cleanup_reconcile import branch_worktree_folder, read_local_worktree_state
from reviewer_worktree import parse_dirty_tracked_files from reviewer_worktree import parse_dirty_tracked_files, REVIEW_WORKTREE_RE
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"}) PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
DEFAULT_TTL_HOURS = float(os.environ.get("GITEA_WORKTREE_TTL_HOURS", "24") or 24) DEFAULT_TTL_HOURS = float(os.environ.get("GITEA_WORKTREE_TTL_HOURS", "24") or 24)
@@ -508,10 +508,8 @@ DISPOSITIONS = frozenset({
"unsafe_unknown", "unsafe_unknown",
}) })
REVIEW_WORKTREE_RE = re.compile(
r"branches/(?:review-pr\d+|merge-simulation-pr\d+|review-[\w-]+)",
re.IGNORECASE,
)
def normalize_path(path: str) -> str: def normalize_path(path: str) -> str: