Compare commits

...
Author SHA1 Message Date
sysadmin 4a63578003 fix(profiles): canonical reconciler ops and guard raw branch delete (#687)
Address REQUEST_CHANGES on PR #688:

- migrate_profiles: emit fully canonical reconciler/author/reviewer defaults;
  canonicalize explicit ops; fail if reconciler loses required pr.close/read
- gitea_delete_branch: deny reconciler (and non-author roles); refuse
  preservation/protected branches before any API call
- gitea_cleanup_merged_pr_branch: require reconciler role only; block
  preservation/evidence branches via branch_cleanup_guard
- docs: end-to-end operator runbook for profile migrate/apply/reconnect/cleanup
- tests: migration canonicalization, idempotency, raw-delete denial, guarded
  cleanup success and unmerged/preserve rejections

Closes #687.
2026-07-12 22:35:14 -04:00
sysadmin c7a444eb4b feat(profiles): support reconciler role in migrate_profiles (Closes #687) 2026-07-12 21:32:01 -04:00
sysadminandClaude Opus 4.8 8cac50b2e7 feat(profiles): make gitea.branch.delete a documented reconciler-owned capability
Merged-PR source-branch cleanup is reconciler work (task_capability_map maps
cleanup_merged_pr_branch -> reconciler / gitea.branch.delete), but the
reconciler profile schema, execution-profile docs, and tests never covered
the permission, so no configured profile could run the guarded
gitea_cleanup_merged_pr_branch path.

- reconciler_profile.py: add gitea.branch.delete to
  RECONCILER_RECOMMENDED_OPERATIONS (not required; not forbidden)
- docs/gitea-execution-profiles.md: document merged-branch cleanup
  ownership, least-privilege constraints, and the no-alias caveat
- tests/test_reconciler_profile.py: reconciler profile with branch.delete
  stays valid and classified reconciler; missing grant is reported as
  missing-recommended
- tests/test_branch_cleanup_guard.py: author- and merger-shaped profiles
  without gitea.branch.delete fail closed on gitea_cleanup_merged_pr_branch

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-12 21:31:00 -04:00
sysadmin 56f1230a10 Merge pull request 'feat(guard): harden workflow against unattributed root WIP and pytest-disabled production guards (Closes #683)' (#684) from fix/issue-683-workflow-guard-hardening into master 2026-07-12 18:58:55 -05:00
sysadmin d302602567 feat(guard): harden workflow against unattributed root WIP and pytest-disabled production guards (Closes #683)
Add shared workflow_scope_guard with typed blockers (blocker_kind +
exact_next_action) and force-on production enforcement under pytest.
Wire root/branches/scope checks through verify_preflight_purity and
mutation entrypoints (create_issue, comment_issue) without adopting the
rejected 300a4ca patterns (test-mode early-return, porcelain *.py filter).

Regression suite covers out-of-scope issue ownership, root diagnostic
edits, worktree bind success path, force-on under pytest, porcelain
integrity, monkeypatch resistance, real entrypoint fail-closed proof,
and durable failure recording.
2026-07-12 13:15:58 -04:00
17 changed files with 2354 additions and 152 deletions
+18
View File
@@ -7,6 +7,19 @@ from typing import Any
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"}) PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
# Evidence / preservation branches must never be removed by cleanup tools
# (e.g. chore/issue-681-preserve-review-session-wip).
_PRESERVATION_MARKERS = ("preserve", "preservation", "evidence")
def is_preservation_or_evidence_branch(branch: str | None) -> bool:
"""Return True when *branch* is a preservation/evidence ref that must stay."""
if not branch:
return False
name = str(branch).lower()
return any(marker in name for marker in _PRESERVATION_MARKERS)
_RAW_BRANCH_DELETE_PATTERNS = ( _RAW_BRANCH_DELETE_PATTERNS = (
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+branch\s+-[dD]\b[^\n\r]*", re.I), re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+branch\s+-[dD]\b[^\n\r]*", re.I),
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+push\b[^\n\r]*\s--delete\b[^\n\r]*", re.I), re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+push\b[^\n\r]*\s--delete\b[^\n\r]*", re.I),
@@ -72,6 +85,11 @@ def assess_merged_pr_branch_cleanup(
reasons.append("PR head branch is missing") reasons.append("PR head branch is missing")
if head_branch in protected: if head_branch in protected:
reasons.append(f"branch '{head_branch}' is protected") reasons.append(f"branch '{head_branch}' is protected")
if is_preservation_or_evidence_branch(head_branch):
reasons.append(
f"branch '{head_branch}' is a preservation/evidence branch and "
"cannot be deleted through merged-PR cleanup"
)
if head_branch in open_pr_heads: if head_branch in open_pr_heads:
reasons.append("an open PR still references this head branch") reasons.append("an open PR still references this head branch")
if head_on_target is False: if head_on_target is False:
+185
View File
@@ -238,11 +238,43 @@ narrow operation set:
- `gitea.issue.comment` - `gitea.issue.comment`
- `gitea.issue.close` - `gitea.issue.close`
- `gitea.pr.close` - `gitea.pr.close`
- `gitea.branch.delete` (merged-branch cleanup only — see below)
Forbidden on reconciler profiles: `gitea.pr.approve`, `gitea.pr.merge`, Forbidden on reconciler profiles: `gitea.pr.approve`, `gitea.pr.merge`,
`gitea.pr.review`, `gitea.pr.create`, `gitea.branch.push`, and `gitea.pr.review`, `gitea.pr.create`, `gitea.branch.push`, and
`gitea.repo.commit`. `gitea.repo.commit`.
### Merged-branch cleanup ownership (`gitea.branch.delete`)
The reconciler is the repository-supported owner of merged-PR source-branch
cleanup: `task_capability_map` maps `cleanup_merged_pr_branch` (and
`reconciliation_cleanup`) to role `reconciler` with permission
`gitea.branch.delete`. Post-merge branch lifecycle is reconciliation work —
it happens after the author, reviewer, and merger roles have completed, and
it must not be reachable from those roles.
Least-privilege constraints:
- `gitea.branch.delete` is granted **only** to reconciler profiles. Author,
reviewer, and merger profiles must never hold it; `gitea_delete_branch`
and `gitea_cleanup_merged_pr_branch` fail closed on any profile without
the permission.
- Even with the permission, reconciler deletion is only supported through the
guarded `gitea_cleanup_merged_pr_branch` path (#514 / #687): the PR must be
merged, the head an ancestor of the target, the branch not protected
(`master`/`main`/`dev`), the branch not a preservation/evidence ref (e.g.
`chore/issue-681-preserve-review-session-wip`), no open PR may still use the
head, and an explicit `CLEANUP MERGED PR <n> BRANCH <branch>` confirmation is
required. Raw `gitea_delete_branch` is **denied** to reconciler even when
`gitea.branch.delete` is present.
- Raw `git branch -d` / `git push --delete` cleanup remains blocked by
`branch_cleanup_guard` and the final-report validator regardless of
profile permissions.
- `gitea.branch.delete` has no short alias in `GITEA_OPERATION_ALIASES`;
write it fully qualified in `allowed_operations`. Migration must emit
canonical names such as `gitea.pr.close` (never bare `pr.close` /
`issue.close`, which the production normalizer rejects or drops).
Launch a static `gitea-reconciler` MCP namespace with Launch a static `gitea-reconciler` MCP namespace with
`GITEA_MCP_PROFILE=prgs-reconciler`. Profile shape is validated by `GITEA_MCP_PROFILE=prgs-reconciler`. Profile shape is validated by
`reconciler_profile.assess_reconciler_profile` (#304). Use the `reconciler_profile.assess_reconciler_profile` (#304). Use the
@@ -251,6 +283,159 @@ Launch a static `gitea-reconciler` MCP namespace with
fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose
heads are not already landed cannot be closed through this path. heads are not already landed cannot be closed through this path.
### Operational runbook: grant reconciler `gitea.branch.delete` (#687)
Merging a code PR that updates `migrate_profiles.py` / `reconciler_profile.py`
**does not** change the live operator profile on disk. Apply the profile
change deliberately, then reconnect the client-managed namespace.
1. **Approved migration / profile-update command** (from the repo root, using
the project venv if present):
```bash
# Dry-run first (default): validates v2 output, writes nothing
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json
# Apply: creates backup then writes migrated v2 config
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json -w
# Optional explicit paths:
# python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json \
# -o ~/.config/gitea-tools/profiles.json \
# --backup ~/.config/gitea-tools/profiles.json.bak -w
```
If the live file is already v2, edit the reconciler identitys
`allowed_operations` / `forbidden_operations` under
`environments.<env>.services.gitea.identities.reconciler` (or the
`prgs-reconciler` alias target) so allowed includes the canonical set
below — then re-validate with a load of the config (see step 3).
2. **Inspect the generated (or edited) profile** — confirm the reconciler
identity, for example:
```bash
python3 - <<'PY'
import json
from pathlib import Path
cfg = json.loads(Path.home().joinpath(".config/gitea-tools/profiles.json").read_text())
# v2 environments shape:
ident = cfg["environments"]["prgs"]["services"]["gitea"]["identities"]["reconciler"]
print("role:", ident.get("role"))
print("allowed:", ident.get("allowed_operations"))
print("forbidden:", ident.get("forbidden_operations"))
PY
```
3. **Validate canonical operation names and least privilege**
Expected canonical **allowed** (defaults after migration):
- `gitea.read`
- `gitea.pr.close` (required)
- `gitea.pr.comment`
- `gitea.issue.comment`
- `gitea.issue.close`
- `gitea.branch.delete` (recommended; cleanup only)
Expected **forbidden** includes at least: `gitea.pr.approve`,
`gitea.pr.merge`, `gitea.pr.review`, `gitea.pr.create`,
`gitea.branch.push`, `gitea.repo.commit`.
No shorthand (`pr.close`, `issue.close`, `pr.comment`) may remain.
Validate with the production loader:
```bash
python3 - <<'PY'
import gitea_config, reconciler_profile
from pathlib import Path
path = str(Path.home() / ".config/gitea-tools/profiles.json")
gitea_config.load_config(path) # fails closed on invalid config
# Or assess the reconciler lists directly after extracting them:
# print(reconciler_profile.assess_reconciler_profile(allowed, forbidden))
PY
```
4. **Merging PR #688 (or any code PR) does not update the live profile.**
Code changes only the migration helper, schema, docs, and tests. The
operator must still run `migrate_profiles.py -w` or an equivalent
authorized edit of `~/.config/gitea-tools/profiles.json`.
5. **Supported apply method:** `python3 migrate_profiles.py … -w` (backup
created automatically) **or** operator-authorized edit of the live
profiles file after backup. Unsupported: silent mtime tricks, manual
process kill to “reload”, or undocumented env overrides.
6. **Backup and validation:** `-w` copies the input to
`<input_path>.bak` (or `--backup PATH`) before writing. Re-run
`load_config` / `assess_reconciler_profile` after write. Keep the
`.bak` until live whoami/capability checks pass.
7. **Client-managed namespace reconnect/reload:** reconnect or reload the
IDE MCP client so `gitea-reconciler` restarts from current `master` and
the updated `GITEA_MCP_PROFILE=prgs-reconciler` config. Do not hand-launch
`mcp_server.py` / `gitea_mcp_server.py` with ad hoc `GITEA_*` env
(see #686 / #630).
8. **Live reverification** (through the client-managed `gitea-reconciler`
namespace only):
- `gitea_whoami` → identity + profile `prgs-reconciler`
- `gitea_assess_master_parity` → `stale=false`, `restart_required=false`
- `gitea_resolve_task_capability(task="cleanup_merged_pr_branch")` →
`allowed_in_current_session=true` only when permission and role match
- `gitea_resolve_task_capability(task="delete_branch")` →
**not** allowed for reconciler (role denial must be enforced)
9. **Guarded cleanup usage** (example for a merged PR whose source branch
remains on the remote):
```text
gitea_cleanup_merged_pr_branch(
pr_number=<N>,
branch=<exact PR head branch>,
confirmation="CLEANUP MERGED PR <N> BRANCH <exact PR head branch>",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="<path under branches/>",
)
```
The tool refuses unmerged PRs, protected branches, preservation/evidence
branches, open-PR heads, mismatched branch names, and wrong confirmation.
10. **Prohibitions**
- No raw `git push --delete`, `git branch -d` / `-D`, or delete refspecs
- No arbitrary `gitea_delete_branch` from reconciler
- No unsupported profile switching mid-run without full re-preflight
- No ad hoc hand-edits of live profiles **unless** operator-authorized,
backed up, and revalidated as above
Canonical migrated reconciler example:
```json
{
"role": "reconciler",
"allowed_operations": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete"
],
"forbidden_operations": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit"
]
}
```
## Identity and fail-closed rules ## Identity and fail-closed rules
Before **any** mutating action, a workflow must know both: Before **any** mutating action, a workflow must know both:
+367 -94
View File
@@ -623,101 +623,280 @@ def verify_preflight_purity(
remote: str | None = None, remote: str | None = None,
worktree_path: str | None = None, worktree_path: str | None = None,
task: str | None = None, task: str | None = None,
*,
target_issue_number: int | None = None,
require_author_lock: bool = False,
): ):
"""Verify that identity and capability were verified prior to session edits.""" """Verify identity/capability order, then production workspace guards.
#683: pytest/unittest must not skip production root/branches/scope
enforcement when force-on signals request production behavior. The
early return below only skips *preflight-order* purity checks under
pure unit-test isolation never when production guards are active.
"""
global _preflight_reviewer_violation_files global _preflight_reviewer_violation_files
in_test = _preflight_in_test_mode() in_test = _preflight_in_test_mode()
if in_test and not ( production_active = workflow_scope_guard.production_guards_active(
os.environ.get("GITEA_TEST_FORCE_DIRTY") in_test_mode=in_test
or os.environ.get("GITEA_TEST_PORCELAIN") is not None )
): # Pure unit-test isolation: skip purity-order unless legacy dirty/porcelain
return # force flags request the dirtiness path. #683 FORCE_PRODUCTION_GUARDS alone
# runs production root/branches/scope without requiring whoami/capability.
skip_purity_order = in_test and not workflow_scope_guard.purity_order_forced()
if not _preflight_whoami_called: if not skip_purity_order:
raise RuntimeError( if not _preflight_whoami_called:
"Pre-flight order violation: Identity (gitea_whoami) has not been verified (fail closed)" raise RuntimeError(
) "Pre-flight order violation: Identity (gitea_whoami) has not been verified (fail closed)"
if not _preflight_capability_called:
raise RuntimeError(
"Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)"
)
if (
task is not None
and _preflight_resolved_task is not None
and task != _preflight_resolved_task
):
raise RuntimeError(
"Pre-flight task mismatch: "
f"resolved '{_preflight_resolved_task}' but mutation requires "
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)
workspace = ctx["workspace_path"]
canonical_root = ctx["canonical_repo_root"]
process_root = ctx["process_project_root"]
real_workspace = os.path.realpath(workspace)
role = ctx.get("workspace_role_kind") or _effective_workspace_role()
if real_workspace != process_root:
if not _preflight_in_test_mode():
membership = author_mutation_worktree.assess_workspace_repo_membership(
workspace_path=workspace,
canonical_repo_root=canonical_root,
) )
if membership["block"]: if not _preflight_capability_called:
raise RuntimeError(
"Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)"
)
if (
task is not None
and _preflight_resolved_task is not None
and task != _preflight_resolved_task
):
raise RuntimeError(
"Pre-flight task mismatch: "
f"resolved '{_preflight_resolved_task}' but mutation requires "
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)
workspace = ctx["workspace_path"]
canonical_root = ctx["canonical_repo_root"]
process_root = ctx["process_project_root"]
real_workspace = os.path.realpath(workspace)
role = ctx.get("workspace_role_kind") or _effective_workspace_role()
if real_workspace != process_root:
if not _preflight_in_test_mode():
membership = author_mutation_worktree.assess_workspace_repo_membership(
workspace_path=workspace,
canonical_repo_root=canonical_root,
)
if membership["block"]:
raise RuntimeError(
author_mutation_worktree.format_workspace_repo_membership_error(
membership
)
)
dirty_files = sorted(
_parse_porcelain_entries(_get_workspace_porcelain(workspace))
)
if dirty_files:
raise RuntimeError( raise RuntimeError(
author_mutation_worktree.format_workspace_repo_membership_error( nwb.format_namespace_workspace_binding_error(
membership role_kind=role,
workspace_path=workspace,
binding_source=ctx.get("workspace_binding_source")
or "unknown binding source",
dirty_files=dirty_files,
ignored_bindings=ctx.get("ignored_bindings"),
) )
) )
else:
dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace))) if _preflight_whoami_violation:
if dirty_files:
raise RuntimeError(
nwb.format_namespace_workspace_binding_error(
role_kind=role,
workspace_path=workspace,
binding_source=ctx.get("workspace_binding_source")
or "unknown binding source",
dirty_files=dirty_files,
ignored_bindings=ctx.get("ignored_bindings"),
)
)
else:
if _preflight_whoami_violation:
raise RuntimeError(
"Pre-flight order violation: Workspace file edits occurred before "
f"gitea_whoami verification (fail closed). Offending files: "
f"{_format_preflight_files(_preflight_whoami_violation_files)}"
)
if _preflight_capability_violation:
raise RuntimeError(
"Pre-flight order violation: Workspace file edits occurred before "
f"gitea_resolve_task_capability verification (fail closed). Offending files: "
f"{_format_preflight_files(_preflight_capability_violation_files)}"
)
if role in {"reviewer", "merger"}:
current = _get_workspace_porcelain()
baseline = _preflight_capability_baseline_porcelain or ""
reviewer_delta = _new_tracked_changes_since(baseline, current)
_preflight_reviewer_violation_files = reviewer_delta
if reviewer_delta:
raise RuntimeError( raise RuntimeError(
f"{role.title()} role violation: profile is forbidden from modifying " "Pre-flight order violation: Workspace file edits occurred before "
"tracked workspace files (fail closed). Offending files: " f"gitea_whoami verification (fail closed). Offending files: "
f"{_format_preflight_files(reviewer_delta)}" f"{_format_preflight_files(_preflight_whoami_violation_files)}"
)
if _preflight_capability_violation:
raise RuntimeError(
"Pre-flight order violation: Workspace file edits occurred before "
f"gitea_resolve_task_capability verification (fail closed). Offending files: "
f"{_format_preflight_files(_preflight_capability_violation_files)}"
) )
_enforce_root_checkout_guard(worktree_path) if role in {"reviewer", "merger"}:
_enforce_branches_only_author_mutation(worktree_path) current = _get_workspace_porcelain()
_clear_preflight_capability_state() baseline = _preflight_capability_baseline_porcelain or ""
reviewer_delta = _new_tracked_changes_since(baseline, current)
_preflight_reviewer_violation_files = reviewer_delta
if reviewer_delta:
raise RuntimeError(
f"{role.title()} role violation: profile is forbidden from modifying "
"tracked workspace files (fail closed). Offending files: "
f"{_format_preflight_files(reviewer_delta)}"
)
# Historical path: root + branches after purity-order when dirty paths live.
_enforce_root_checkout_guard(worktree_path)
_enforce_branches_only_author_mutation(worktree_path)
_enforce_issue_scope_guard(
worktree_path,
task=task,
target_issue_number=target_issue_number,
require_author_lock=require_author_lock,
)
_clear_preflight_capability_state()
return
# #683: under pytest unit isolation, FORCE_PRODUCTION_GUARDS still runs
# production root + branches + issue scope (no silent no-op of guards).
if production_active:
_enforce_root_checkout_guard(worktree_path)
_enforce_branches_only_author_mutation(worktree_path)
_enforce_issue_scope_guard(
worktree_path,
task=task,
target_issue_number=target_issue_number,
require_author_lock=require_author_lock,
)
def _session_issue_lock_snapshot(
workspace_path: str | None = None,
) -> dict:
"""Return session lock fields relevant to #683 scope enforcement.
Branch-vs-lock comparison uses the live workspace branch only when the
lock's worktree matches the mutation workspace. That prevents a foreign
or leftover session lock from poisoning unrelated test worktrees while
still fail-closing when the bound worktree drifts to another issue.
"""
lock = issue_lock_store.read_session_issue_lock() or {}
raw = lock.get("issue_number")
locked: int | None
try:
locked = int(raw) if raw is not None else None
except (TypeError, ValueError):
locked = None
lock_wt = (lock.get("worktree_path") or "").strip()
workspace = (workspace_path or "").strip()
worktrees_match = False
if lock_wt and workspace:
try:
worktrees_match = os.path.realpath(lock_wt) == os.path.realpath(workspace)
except OSError:
worktrees_match = False
return {
"locked_issue_number": locked,
"lock_branch_name": (lock.get("branch_name") or "").strip() or None,
"lock_worktree_path": lock_wt or None,
"worktrees_match": worktrees_match,
}
def _session_locked_issue_number() -> int | None:
"""Return the active session issue lock number when present (#683)."""
return _session_issue_lock_snapshot().get("locked_issue_number")
def _enforce_issue_scope_guard(
worktree_path: str | None = None,
*,
task: str | None = None,
target_issue_number: int | None = None,
require_author_lock: bool = False,
) -> None:
"""#683: fail closed on missing/out-of-scope issue ownership for mutations."""
ctx = _resolve_namespace_mutation_context(worktree_path)
workspace = ctx["workspace_path"]
git_state = issue_lock_worktree.read_worktree_git_state(workspace)
# Honour actual profile role as well as poisoned task role (#540 / #683):
# comment_issue preflight stamps required_role_kind=author, which must not
# strip a genuine reconciler of control-checkout exemptions.
role = ctx.get("workspace_role_kind") or _effective_workspace_role()
actual = _actual_profile_role()
if actual in nwb.NON_AUTHOR_ROLES:
role = actual
snap = _session_issue_lock_snapshot(workspace)
# Scope uses the lock's recorded branch for issue-number matching.
# Live workspace branch can inherit the parent control checkout's branch
# name when a temp branches/ dir is not its own worktree tip — that must
# not invent a false out-of-scope failure. Live branch drift is enforced
# by issue_lock_store.verify_lock_for_mutation elsewhere.
branch_for_scope = snap.get("lock_branch_name")
if (
snap.get("worktrees_match")
and workflow_scope_guard.production_guards_forced()
):
live_branch = git_state.get("current_branch")
live_issue = workflow_scope_guard.extract_issue_number_from_branch(
live_branch
)
locked = snap.get("locked_issue_number")
if (
live_issue is not None
and locked is not None
and live_issue != locked
):
branch_for_scope = live_branch
# Author implementation / source-adjacent mutations need ownership when forced.
authorish = role == "author" or (
task
in {
"create_issue",
"comment_issue",
"lock_issue",
"create_pr",
"commit_files",
"gitea_commit_files",
"mark_issue",
}
)
require_lock = bool(require_author_lock) or (
authorish
and workflow_scope_guard.production_guards_forced()
and role == "author"
)
assessment = workflow_scope_guard.assess_production_mutation_guards(
workspace_path=workspace,
canonical_repo_root=ctx["canonical_repo_root"],
porcelain_status=git_state.get("porcelain_status") or "",
current_branch=branch_for_scope,
locked_issue_number=snap.get("locked_issue_number"),
target_issue_number=target_issue_number,
role_kind=role,
require_author_lock=require_lock,
in_test_mode=_preflight_in_test_mode(),
)
workflow_scope_guard.raise_if_blocked(assessment)
def _production_guard_block_from_exc(exc: BaseException, **extra) -> dict | None:
"""Map production-guard exceptions to typed tool block responses (#683)."""
if isinstance(exc, workflow_scope_guard.ProductionGuardError):
return workflow_scope_guard.block_response(exc, **extra)
text = str(exc)
if "Workflow scope guard (#683)" in text or "Root checkout guard (#475)" in text:
kind = workflow_scope_guard.BLOCKER_PRODUCTION_GUARD
if "root_diagnostic_edit" in text or "tracked source or test edits" in text:
kind = workflow_scope_guard.BLOCKER_ROOT_DIAGNOSTIC_EDIT
elif "Branches-only mutation guard" in text or "stable control checkout" in text:
kind = workflow_scope_guard.BLOCKER_MISSING_WORKTREE
elif "out-of-scope" in text or "locked to issue" in text:
kind = workflow_scope_guard.BLOCKER_OUT_OF_SCOPE_ISSUE
elif "no owning issue" in text:
kind = workflow_scope_guard.BLOCKER_MISSING_ISSUE_SCOPE
return workflow_scope_guard.block_response(
blocker_kind=kind,
reasons=[text],
**extra,
)
if "Branches-only mutation guard" in text:
return workflow_scope_guard.block_response(
blocker_kind=workflow_scope_guard.BLOCKER_MISSING_WORKTREE,
reasons=[text],
**extra,
)
if "Root checkout guard" in text:
return workflow_scope_guard.block_response(
blocker_kind=workflow_scope_guard.BLOCKER_ROOT_DIAGNOSTIC_EDIT,
reasons=[text],
**extra,
)
return None
def _verify_role_mutation_workspace( def _verify_role_mutation_workspace(
@@ -727,7 +906,12 @@ def _verify_role_mutation_workspace(
worktree: str | None = None, worktree: str | None = None,
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).
#683: must NOT early-return solely because pytest/unittest is loaded.
Production workspace binding always runs; test isolation uses explicit
env fixtures / force-on flags, never a production short-circuit here.
"""
# Check running runtimes to prevent stale mutations # Check running runtimes to prevent stale mutations
try: try:
@@ -928,6 +1112,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 workflow_scope_guard # noqa: E402 # #683 production scope / force-on guards
import stable_branch_push_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
@@ -1896,7 +2081,15 @@ def gitea_create_issue(
) )
if blocked: if blocked:
return blocked return blocked
verify_preflight_purity(remote, worktree_path=worktree_path, task="create_issue") try:
verify_preflight_purity(
remote, worktree_path=worktree_path, task="create_issue"
)
except Exception as exc:
typed = _production_guard_block_from_exc(exc, number=None)
if typed is not None:
return typed
raise
content_gate = issue_content_gate.pre_create_issue_content_gate( content_gate = issue_content_gate.pre_create_issue_content_gate(
title, title,
body, body,
@@ -5732,6 +5925,65 @@ def gitea_delete_branch(
"permission_report": _permission_block_report("gitea.branch.delete"), "permission_report": _permission_block_report("gitea.branch.delete"),
} }
# Possessing gitea.branch.delete alone is not enough for arbitrary deletion.
# task_capability_map maps delete_branch → author; reconciler must use the
# guarded cleanup_merged_pr_branch path only (#687 / #514).
profile = get_profile()
active_role = _profile_role_kind(profile)
required_role = task_capability_map.required_role("delete_branch")
if active_role == "reconciler":
return {
"success": False,
"performed": False,
"required_permission": "gitea.branch.delete",
"required_role_kind": required_role,
"active_role_kind": active_role,
"reasons": [
"reconciler profile cannot use raw gitea_delete_branch; "
"use gitea_cleanup_merged_pr_branch for a fully merged PR "
"source branch only (fail closed)"
],
"exact_next_action": (
"Call gitea_cleanup_merged_pr_branch with pr_number, the "
"exact PR head branch, and confirmation "
"'CLEANUP MERGED PR <n> BRANCH <branch>' after capability "
"resolve for cleanup_merged_pr_branch."
),
"permission_report": _permission_block_report("gitea.branch.delete"),
}
if active_role != required_role:
return {
"success": False,
"performed": False,
"required_permission": "gitea.branch.delete",
"required_role_kind": required_role,
"active_role_kind": active_role,
"reasons": [
f"Active profile role '{active_role}' cannot perform "
f"{required_role} task 'delete_branch' even when "
"gitea.branch.delete is present (fail closed)"
],
"permission_report": _permission_block_report("gitea.branch.delete"),
}
if branch_cleanup_guard.is_preservation_or_evidence_branch(branch):
return {
"success": False,
"performed": False,
"required_permission": "gitea.branch.delete",
"reasons": [
f"branch '{branch}' is a preservation/evidence branch and "
"cannot be deleted (fail closed)"
],
}
if branch in branch_cleanup_guard.PROTECTED_BRANCHES:
return {
"success": False,
"performed": False,
"required_permission": "gitea.branch.delete",
"reasons": [f"branch '{branch}' is protected (fail closed)"],
}
audit_allowed, audit_reasons = ( audit_allowed, audit_reasons = (
audit_reconciliation_mode.check_audit_mutation_allowed("delete_branch") audit_reconciliation_mode.check_audit_mutation_allowed("delete_branch")
) )
@@ -5783,18 +6035,20 @@ def gitea_cleanup_merged_pr_branch(
} }
profile = get_profile() profile = get_profile()
active_role = _role_kind( active_role = _profile_role_kind(profile)
profile.get("allowed_operations", []), # cleanup_merged_pr_branch is reconciler-owned (task_capability_map).
profile.get("forbidden_operations", []), # Author/reviewer/merger must not reach this path even if they somehow
) # hold gitea.branch.delete.
if active_role == "reviewer": if active_role != "reconciler":
return { return {
"success": False, "success": False,
"performed": False, "performed": False,
"required_permission": "gitea.branch.delete", "required_permission": "gitea.branch.delete",
"required_role_kind": "reconciler",
"active_role_kind": active_role,
"reasons": [ "reasons": [
"reviewer profile is not authorized for merged branch cleanup " f"profile role '{active_role}' is not authorized for merged "
"(fail closed)" "branch cleanup; required role is reconciler (fail closed)"
], ],
"permission_report": _permission_block_report("gitea.branch.delete"), "permission_report": _permission_block_report("gitea.branch.delete"),
} }
@@ -8167,9 +8421,26 @@ def gitea_create_issue_comment(
with the reveal opt-in); on a permission block or empty body, with the reveal opt-in); on a permission block or empty body,
'success'/'performed' False and 'reasons' with no API call made 'success'/'performed' False and 'reasons' with no API call made
(permission blocks also carry a structured 'permission_report', (permission blocks also carry a structured 'permission_report',
#142). #142). On production-guard blocks (#683): 'blocker_kind' and
'exact_next_action' with no API side effect.
""" """
verify_preflight_purity(remote, worktree_path=worktree_path, task="comment_issue") try:
# Do not pass target_issue_number: comments on other issues remain
# allowed while an author holds a different implementation lock.
# Scope ownership for source edits is enforced via branch/worktree
# binding + root diagnostic checks (#683).
verify_preflight_purity(
remote,
worktree_path=worktree_path,
task="comment_issue",
)
except Exception as exc:
typed = _production_guard_block_from_exc(
exc, issue_number=issue_number
)
if typed is not None:
return typed
raise
gate_reasons = _profile_operation_gate("gitea.issue.comment") gate_reasons = _profile_operation_gate("gitea.issue.comment")
reasons = list(gate_reasons) reasons = list(gate_reasons)
if not (body or "").strip(): if not (body or "").strip():
@@ -11059,6 +11330,8 @@ def gitea_resolve_task_capability(
"gitea_commit_files", "gitea_commit_files",
"address_pr_change_requests", "address_pr_change_requests",
"delete_branch", "delete_branch",
"cleanup_merged_pr_branch",
"reconciliation_cleanup",
"work_issue", "work_issue",
"work-issue", "work-issue",
} }
+127 -8
View File
@@ -20,12 +20,114 @@ if PROJECT_ROOT not in sys.path:
import gitea_config import gitea_config
AUTHOR_DEFAULT_ALLOWED = ["read", "branch", "commit", "push", "open_pr", "comment"] # Defaults emit *canonical* operation names only. Shorthand that is not in
AUTHOR_DEFAULT_FORBIDDEN = ["approve", "request_changes", "merge"] # gitea_config.GITEA_OPERATION_ALIASES (e.g. ``pr.close``, ``issue.close``)
REVIEWER_DEFAULT_ALLOWED = [ # is silently dropped by the production loader and must never appear here.
"read", "review", "comment", "approve", "request_changes", "merge" AUTHOR_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
"gitea.pr.comment",
] ]
REVIEWER_DEFAULT_FORBIDDEN = ["branch", "commit", "push", "open_pr"] AUTHOR_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
]
REVIEWER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.review",
"gitea.pr.comment",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
]
REVIEWER_DEFAULT_FORBIDDEN = [
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
]
# Required reconciler ops (read + pr.close) plus recommended comment/close and
# branch.delete for guarded merged-PR cleanup. All names must normalize via
# gitea_config.normalize_operation without being dropped.
RECONCILER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
]
RECONCILER_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit",
]
# Migration-only expansions for common shorthands that are *not* in
# GITEA_OPERATION_ALIASES. Emitted output is always the canonical form so a
# second canonicalize pass is a no-op (idempotent).
_MIGRATION_ONLY_ALIASES = {
"pr.close": "gitea.pr.close",
"pr.comment": "gitea.pr.comment",
"issue.close": "gitea.issue.close",
"branch.delete": "gitea.branch.delete",
}
# Reconciler required ops that must survive migration (from reconciler_profile).
RECONCILER_REQUIRED_CANONICAL = ("gitea.read", "gitea.pr.close")
def canonicalize_operation(op: str) -> str:
"""Return a canonical operation name accepted by the production loader.
Fail closed on unknown/ambiguous spellings so required permissions cannot
be silently dropped by ``check_operation`` later.
"""
if not isinstance(op, str) or not op.strip():
raise ValueError("operation must be a non-empty string (fail closed)")
op = op.strip()
try:
return gitea_config.normalize_operation(op)
except gitea_config.ConfigError:
pass
if op in _MIGRATION_ONLY_ALIASES:
return _MIGRATION_ONLY_ALIASES[op]
raise ValueError(
f"operation {op!r} cannot be canonicalized for migration "
"(unknown/ambiguous; fail closed — production loader would drop it)"
)
def canonicalize_operations(ops, *, context: str = "operations") -> list[str]:
"""Canonicalize a list of operations; preserve order, drop duplicates."""
if not isinstance(ops, list):
raise ValueError(f"{context} must be a list (fail closed)")
out: list[str] = []
seen: set[str] = set()
for entry in ops:
canon = canonicalize_operation(entry)
if canon not in seen:
seen.add(canon)
out.append(canon)
return out
def _assert_reconciler_required_survive(allowed: list[str], profile_name: str) -> None:
"""Fail visibly when migration would leave a reconciler without required ops."""
missing = [op for op in RECONCILER_REQUIRED_CANONICAL if op not in set(allowed)]
if missing:
raise ValueError(
f"Profile '{profile_name}' (reconciler) is missing required "
f"operation(s) after migration: {missing}. Refusing to emit a "
"profile that would silently fail pr.close / read (fail closed)."
)
def infer_role(name, execution_profile): def infer_role(name, execution_profile):
@@ -90,9 +192,11 @@ def migrate_v1_to_v2(v1_data):
ident_name = "reviewer" ident_name = "reviewer"
elif role == "author": elif role == "author":
ident_name = "author" ident_name = "author"
elif role == "reconciler":
ident_name = "reconciler"
else: else:
role = prof.get("role") role = prof.get("role")
if role not in (None, "author", "reviewer"): if role not in (None, "author", "reviewer", "reconciler"):
raise ValueError( raise ValueError(
f"Profile '{name}' has unsupported role {role!r}" f"Profile '{name}' has unsupported role {role!r}"
) )
@@ -124,20 +228,35 @@ def migrate_v1_to_v2(v1_data):
raise ValueError( raise ValueError(
f"Profile '{name}' operation fields must be lists" f"Profile '{name}' operation fields must be lists"
) )
identity_data["allowed_operations"] = list(allowed) try:
identity_data["forbidden_operations"] = list(forbidden) identity_data["allowed_operations"] = canonicalize_operations(
allowed, context=f"profile '{name}' allowed_operations"
)
identity_data["forbidden_operations"] = canonicalize_operations(
forbidden, context=f"profile '{name}' forbidden_operations"
)
except ValueError as exc:
raise ValueError(f"Profile '{name}': {exc}") from exc
elif role == "author": elif role == "author":
identity_data["allowed_operations"] = list(AUTHOR_DEFAULT_ALLOWED) identity_data["allowed_operations"] = list(AUTHOR_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(AUTHOR_DEFAULT_FORBIDDEN) identity_data["forbidden_operations"] = list(AUTHOR_DEFAULT_FORBIDDEN)
elif role == "reviewer": elif role == "reviewer":
identity_data["allowed_operations"] = list(REVIEWER_DEFAULT_ALLOWED) identity_data["allowed_operations"] = list(REVIEWER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(REVIEWER_DEFAULT_FORBIDDEN) identity_data["forbidden_operations"] = list(REVIEWER_DEFAULT_FORBIDDEN)
elif role == "reconciler":
identity_data["allowed_operations"] = list(RECONCILER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(RECONCILER_DEFAULT_FORBIDDEN)
else: else:
raise ValueError( raise ValueError(
f"Profile '{name}' has no explicit operation lists and no " f"Profile '{name}' has no explicit operation lists and no "
"unambiguous author/reviewer role marker (fail closed)" "unambiguous author/reviewer role marker (fail closed)"
) )
if role == "reconciler":
_assert_reconciler_required_survive(
identity_data["allowed_operations"], name
)
# Nest inside environments/services structure # Nest inside environments/services structure
env = environments.setdefault(env_name, {}) env = environments.setdefault(env_name, {})
services = env.setdefault("services", {}) services = env.setdefault("services", {})
+5
View File
@@ -18,6 +18,11 @@ RECONCILER_RECOMMENDED_OPERATIONS = (
"gitea.pr.comment", "gitea.pr.comment",
"gitea.issue.comment", "gitea.issue.comment",
"gitea.issue.close", "gitea.issue.close",
# Merged-branch cleanup is reconciler-owned (task_capability_map maps
# cleanup_merged_pr_branch -> reconciler). The permission is only
# exercisable through the guarded gitea_cleanup_merged_pr_branch path
# (#514): merged proof, protected-branch refusal, explicit confirmation.
"gitea.branch.delete",
) )
RECONCILER_FORBIDDEN_OPERATIONS = ( RECONCILER_FORBIDDEN_OPERATIONS = (
+7 -1
View File
@@ -27,7 +27,13 @@ from task_capability_map import required_permission, required_role
DELETE_PROFILE = { DELETE_PROFILE = {
"profile_name": "prgs-author-delete", "profile_name": "prgs-author-delete",
"allowed_operations": ["gitea.read", "gitea.branch.delete"], "role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": [], "forbidden_operations": [],
"audit_label": "prgs-author-delete", "audit_label": "prgs-author-delete",
} }
+227 -16
View File
@@ -8,10 +8,33 @@ import branch_cleanup_guard as guard # noqa: E402
import mcp_server # noqa: E402 import mcp_server # noqa: E402
import task_capability_map # noqa: E402 import task_capability_map # noqa: E402
from final_report_validator import assess_final_report_validator # noqa: E402 from final_report_validator import assess_final_report_validator # noqa: E402
from mcp_server import gitea_cleanup_merged_pr_branch # noqa: E402 from mcp_server import gitea_cleanup_merged_pr_branch, gitea_delete_branch # noqa: E402
FAKE_AUTH = "token fake" FAKE_AUTH = "token fake"
# Reconciler-shaped profile that holds branch.delete (recommended) plus
# required pr.close/read so _role_kind classifies as reconciler.
RECONCILER_WITH_DELETE = {
"profile_name": "prgs-reconciler",
"role": "reconciler",
"allowed_operations": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
],
"forbidden_operations": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit",
],
}
class TestRawBranchDeleteGuard(unittest.TestCase): class TestRawBranchDeleteGuard(unittest.TestCase):
def test_detects_local_and_remote_raw_git_delete_commands(self): def test_detects_local_and_remote_raw_git_delete_commands(self):
@@ -93,14 +116,48 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
self.assertEqual(res["required_permission"], "gitea.branch.delete") self.assertEqual(res["required_permission"], "gitea.branch.delete")
self.mock_api.assert_not_called() self.mock_api.assert_not_called()
def test_author_and_merger_without_delete_authority_fail_closed(self):
role_profiles = {
"author": [
"gitea.read",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
],
"merger": ["gitea.read", "gitea.pr.merge"],
}
for name, allowed in role_profiles.items():
with self.subTest(role=name):
profile_patch = patch(
"mcp_server.get_profile",
return_value={
"profile_name": name,
"allowed_operations": allowed,
"forbidden_operations": [],
},
)
profile_patch.start()
try:
res = gitea_cleanup_merged_pr_branch(
pr_number=487,
confirmation="CLEANUP MERGED PR 487 BRANCH feat/branch",
branch="feat/branch",
remote="prgs",
worktree_path="/tmp/repo/branches/cleanup",
)
finally:
profile_patch.stop()
self.assertFalse(res["performed"])
self.assertEqual(
res["required_permission"], "gitea.branch.delete"
)
self.mock_api.assert_not_called()
def test_root_checkout_cleanup_fails_closed(self): def test_root_checkout_cleanup_fails_closed(self):
patch( patch(
"mcp_server.get_profile", "mcp_server.get_profile",
return_value={ return_value=dict(RECONCILER_WITH_DELETE),
"profile_name": "branch-cleanup",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"forbidden_operations": [],
},
).start() ).start()
res = gitea_cleanup_merged_pr_branch( res = gitea_cleanup_merged_pr_branch(
pr_number=487, pr_number=487,
@@ -117,11 +174,7 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
branch = "feat/issue-485-lease-comments-non-list-guard" branch = "feat/issue-485-lease-comments-non-list-guard"
patch( patch(
"mcp_server.get_profile", "mcp_server.get_profile",
return_value={ return_value=dict(RECONCILER_WITH_DELETE),
"profile_name": "branch-cleanup",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"forbidden_operations": [],
},
).start() ).start()
self.mock_api.side_effect = [ self.mock_api.side_effect = [
{ {
@@ -152,11 +205,7 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
branch = "feat/issue-485-lease-comments-non-list-guard" branch = "feat/issue-485-lease-comments-non-list-guard"
patch( patch(
"mcp_server.get_profile", "mcp_server.get_profile",
return_value={ return_value=dict(RECONCILER_WITH_DELETE),
"profile_name": "branch-cleanup",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"forbidden_operations": [],
},
).start() ).start()
self.mock_api.side_effect = [ self.mock_api.side_effect = [
{ {
@@ -182,6 +231,168 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
] ]
self.assertFalse(delete_calls) self.assertFalse(delete_calls)
def test_reconciler_with_branch_delete_cannot_raw_delete(self):
"""#687: reconciler + gitea.branch.delete still cannot call raw delete."""
patch(
"mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE),
).start()
res = gitea_delete_branch(
branch="fix/issue-683-workflow-guard-hardening",
remote="prgs",
)
self.assertFalse(res.get("success", True))
self.assertFalse(res.get("performed", True))
reasons = " ".join(res.get("reasons") or [])
self.assertIn("raw gitea_delete_branch", reasons)
self.assertIn("cleanup_merged_pr_branch", reasons)
self.mock_api.assert_not_called()
def test_reconciler_raw_delete_denies_preservation_branch(self):
patch(
"mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE),
).start()
res = gitea_delete_branch(
branch="chore/issue-681-preserve-review-session-wip",
remote="prgs",
)
self.assertFalse(res.get("performed", True))
self.mock_api.assert_not_called()
def test_author_with_branch_delete_role_ok_but_preserve_blocked(self):
"""Author role may use raw delete path when permitted; preserve fails closed."""
patch(
"mcp_server.get_profile",
return_value={
"profile_name": "prgs-author",
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"],
},
).start()
res = gitea_delete_branch(
branch="chore/issue-681-preserve-review-session-wip",
remote="prgs",
)
self.assertFalse(res.get("performed", True))
self.assertIn("preservation", " ".join(res.get("reasons") or []))
self.mock_api.assert_not_called()
def test_unmerged_branch_cleanup_rejected(self):
branch = "feat/unmerged-work"
patch(
"mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE),
).start()
self.mock_api.side_effect = [
{
"number": 999,
"merged": False,
"merged_at": None,
"head": {"ref": branch, "sha": "b" * 40},
"base": {"ref": "master"},
},
{},
]
res = gitea_cleanup_merged_pr_branch(
pr_number=999,
confirmation=f"CLEANUP MERGED PR 999 BRANCH {branch}",
branch=branch,
remote="prgs",
worktree_path="/tmp/repo/branches/cleanup",
)
self.assertFalse(res["performed"])
self.assertTrue(
any("not merged" in r for r in (res.get("reasons") or []))
)
delete_calls = [
call for call in self.mock_api.call_args_list if call.args[0] == "DELETE"
]
self.assertFalse(delete_calls)
def test_preservation_branch_cleanup_rejected(self):
branch = "chore/issue-681-preserve-review-session-wip"
patch(
"mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE),
).start()
self.mock_api.side_effect = [
{
"number": 681,
"merged": True,
"merged_at": "2026-07-08T01:00:00Z",
"head": {"ref": branch, "sha": "c" * 40},
"base": {"ref": "master"},
},
{},
]
res = gitea_cleanup_merged_pr_branch(
pr_number=681,
confirmation=f"CLEANUP MERGED PR 681 BRANCH {branch}",
branch=branch,
remote="prgs",
worktree_path="/tmp/repo/branches/cleanup",
)
self.assertFalse(res["performed"])
self.assertTrue(
any("preservation" in r for r in (res.get("reasons") or []))
)
delete_calls = [
call for call in self.mock_api.call_args_list if call.args[0] == "DELETE"
]
self.assertFalse(delete_calls)
def test_non_reconciler_with_delete_denied_cleanup(self):
patch(
"mcp_server.get_profile",
return_value={
"profile_name": "prgs-author",
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": [],
},
).start()
res = gitea_cleanup_merged_pr_branch(
pr_number=487,
confirmation="CLEANUP MERGED PR 487 BRANCH feat/branch",
branch="feat/branch",
remote="prgs",
worktree_path="/tmp/repo/branches/cleanup",
)
self.assertFalse(res["performed"])
self.assertEqual(res.get("required_role_kind"), "reconciler")
self.mock_api.assert_not_called()
def test_assess_guard_rejects_preservation_branch(self):
assessment = guard.assess_merged_pr_branch_cleanup(
pr_number=681,
head_branch="chore/issue-681-preserve-review-session-wip",
merged=True,
remote_branch_exists=True,
open_pr_heads=set(),
head_on_target=True,
delete_capability_allowed=True,
confirmation=(
"CLEANUP MERGED PR 681 BRANCH "
"chore/issue-681-preserve-review-session-wip"
),
)
self.assertFalse(assessment["safe_to_delete"])
self.assertTrue(
any("preservation" in r for r in assessment["block_reasons"])
)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+39 -10
View File
@@ -57,9 +57,23 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
# Without worktree_path/env hints, workspace resolves to PROJECT_ROOT. When that # Without worktree_path/env hints, workspace resolves to PROJECT_ROOT. When that
# path is the stable control checkout (not under branches/), mutation must fail. # path is the stable control checkout (not under branches/), mutation must fail.
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue(title="Test issue", body="body text") res = srv.gitea_create_issue(title="Test issue", body="body text")
self.assertIn("stable control checkout", str(ctx.exception)) except RuntimeError as exc:
self.assertIn("stable control checkout", str(exc))
else:
# #683: production guards return typed blockers at entrypoints
self.assertFalse(res.get("success"))
self.assertFalse(res.get("performed"))
blob = " ".join(res.get("reasons") or []) + " " + str(
res.get("blocker_kind") or ""
)
self.assertTrue(
"stable control checkout" in blob
or "missing_issue_worktree" in blob
or "control checkout" in blob.lower()
)
self.assertTrue(res.get("exact_next_action"))
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@patch("gitea_mcp_server._profile_permission_block", return_value=None) @patch("gitea_mcp_server._profile_permission_block", return_value=None)
@@ -105,11 +119,17 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
) )
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue( res = srv.gitea_create_issue(
title="Test issue", body="body", worktree_path=missing_path title="Test issue", body="body", worktree_path=missing_path
) )
self.assertIn("does not exist (fail closed)", str(ctx.exception)) except RuntimeError as exc:
self.assertIn("does not exist", str(exc))
else:
self.assertFalse(res.get("success"))
blob = " ".join(res.get("reasons") or [])
self.assertIn("does not exist", blob)
self.assertTrue(res.get("exact_next_action") or res.get("reasons"))
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@patch("gitea_mcp_server._profile_permission_block", return_value=None) @patch("gitea_mcp_server._profile_permission_block", return_value=None)
@@ -142,11 +162,20 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""): with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue( res = srv.gitea_create_issue(
title="Test issue", body="body", worktree_path=wrong_repo_path title="Test issue",
body="body",
worktree_path=wrong_repo_path,
) )
self.assertIn("does not belong to the target repository", str(ctx.exception)) except RuntimeError as exc:
self.assertIn(
"does not belong to the target repository", str(exc)
)
else:
self.assertFalse(res.get("success"))
blob = " ".join(res.get("reasons") or [])
self.assertIn("does not belong to the target repository", blob)
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@patch("gitea_mcp_server._profile_permission_block", return_value=None) @patch("gitea_mcp_server._profile_permission_block", return_value=None)
+11 -2
View File
@@ -267,10 +267,19 @@ class TestReconcilerCommentThroughCanonicalPath(unittest.TestCase):
with patch.dict(os.environ, {}, clear=False): with patch.dict(os.environ, {}, clear=False):
os.environ.pop("GITEA_AUTHOR_WORKTREE", None) os.environ.pop("GITEA_AUTHOR_WORKTREE", None)
os.environ.pop("GITEA_ACTIVE_WORKTREE", None) os.environ.pop("GITEA_ACTIVE_WORKTREE", None)
with self.assertRaises(RuntimeError): try:
srv.gitea_create_issue_comment( res = srv.gitea_create_issue_comment(
515, "author note", remote="prgs" 515, "author note", remote="prgs"
) )
except RuntimeError:
pass # legacy raise path
else:
# #683: typed blocker at mutation entrypoint
self.assertFalse(res.get("success"))
self.assertFalse(res.get("performed"))
self.assertTrue(
res.get("blocker_kind") or res.get("reasons")
)
mock_api.assert_not_called() mock_api.assert_not_called()
@@ -0,0 +1,472 @@
"""#683: block unattributed root WIP; pytest cannot disable production guards.
Regression coverage required by issue #683:
1. Session locked to issue A blocks unrelated target issue B until B is selected.
2. Diagnostic source edit on the root checkout is blocked.
3. Same legitimate edit succeeds after issue ownership + isolated worktree bind.
4. Running under pytest does not deactivate production guards when force-on.
5. Dirty tracked Python files remain visible to porcelain consumers.
6. Monkeypatching one helper cannot silently turn the full guard path into a no-op.
7. Real mutation entrypoint proves production guards run before side effects.
8. Same-issue edits in a valid isolated worktree remain unaffected.
9. Blocker includes stable reason + exact recovery action.
"""
from __future__ import annotations
import os
import sys
import tempfile
import textwrap
import unittest
from pathlib import Path
from unittest.mock import MagicMock, patch
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import gitea_mcp_server as mcp_server # noqa: E402
import issue_lock_worktree # noqa: E402
import workflow_scope_guard as wsg # noqa: E402
CONTROL_ROOT = str(Path(__file__).resolve().parent.parent)
if "branches" in Path(__file__).resolve().parts:
# Running from a worktree under branches/ — parent of branches is control.
parts = Path(__file__).resolve().parts
idx = parts.index("branches")
CONTROL_ROOT = str(Path(*parts[:idx])) if idx > 0 else CONTROL_ROOT
class TestProductionGuardsForceOn(unittest.TestCase):
def tearDown(self):
for key in (
wsg.FORCE_PRODUCTION_GUARDS_ENV,
"GITEA_TEST_FORCE_DIRTY",
"GITEA_TEST_PORCELAIN",
"GITEA_AUTHOR_WORKTREE",
"GITEA_ACTIVE_WORKTREE",
):
os.environ.pop(key, None)
wsg.clear_workflow_failure_ledger()
def test_force_on_under_pytest_keeps_production_active(self):
self.assertTrue(wsg.production_guards_active(in_test_mode=False))
self.assertFalse(wsg.production_guards_active(in_test_mode=True))
os.environ[wsg.FORCE_PRODUCTION_GUARDS_ENV] = "1"
self.assertTrue(wsg.production_guards_active(in_test_mode=True))
self.assertTrue(wsg.production_guards_forced())
def test_no_early_return_in_verify_role_mutation_workspace_source(self):
src = Path(mcp_server.__file__).read_text(encoding="utf-8")
# Rejected 300a4ca pattern must not exist.
self.assertNotIn(
"if _preflight_in_test_mode():\n return _resolve_preflight_workspace_path",
src,
)
# Docstring contract for #683.
self.assertIn("#683", src)
self.assertIn("must NOT early-return solely because pytest", src)
class TestPorcelainIntegrity(unittest.TestCase):
def test_read_worktree_git_state_surfaces_dirty_py(self):
with tempfile.TemporaryDirectory() as tmp:
# Use a real git repo so porcelain is truthful.
import subprocess
subprocess.run(["git", "init"], cwd=tmp, check=True, capture_output=True)
subprocess.run(
["git", "config", "user.email", "[email protected]"],
cwd=tmp,
check=True,
capture_output=True,
)
subprocess.run(
["git", "config", "user.name", "t"],
cwd=tmp,
check=True,
capture_output=True,
)
py_path = Path(tmp) / "sample_mod.py"
py_path.write_text("x = 1\n", encoding="utf-8")
subprocess.run(["git", "add", "sample_mod.py"], cwd=tmp, check=True)
subprocess.run(
["git", "commit", "-m", "init"],
cwd=tmp,
check=True,
capture_output=True,
)
py_path.write_text("x = 2\n", encoding="utf-8")
state = issue_lock_worktree.read_worktree_git_state(tmp)
porcelain = state.get("porcelain_status") or ""
self.assertIn("sample_mod.py", porcelain)
self.assertTrue(any(line.strip().endswith(".py") for line in porcelain.splitlines()))
def test_production_reader_source_rejects_pytest_py_filter(self):
src = Path(issue_lock_worktree.__file__).read_text(encoding="utf-8")
findings = wsg.assert_no_pytest_porcelain_filter(src)
self.assertEqual(findings, [])
# Negative: the rejected 300a4ca pattern is detected.
rejected = textwrap.dedent(
"""
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")
)
"""
)
self.assertTrue(wsg.assert_no_pytest_porcelain_filter(rejected))
class TestIssueScopeOwnership(unittest.TestCase):
def test_out_of_scope_issue_blocked_until_selected(self):
result = wsg.assess_issue_scope_ownership(
locked_issue_number=100,
target_issue_number=200,
branch_name="fix/issue-100-example",
role_kind="author",
)
self.assertTrue(result["block"])
self.assertEqual(result["blocker_kind"], wsg.BLOCKER_OUT_OF_SCOPE_ISSUE)
self.assertIn("exact_next_action", result)
self.assertIn("owning issue", result["exact_next_action"].lower())
self.assertTrue(result["reasons"])
def test_same_issue_scope_allowed(self):
result = wsg.assess_issue_scope_ownership(
locked_issue_number=100,
target_issue_number=100,
branch_name="fix/issue-100-example",
role_kind="author",
)
self.assertFalse(result["block"])
self.assertEqual(result["exact_next_action"], "proceed")
def test_missing_lock_when_required(self):
result = wsg.assess_issue_scope_ownership(
locked_issue_number=None,
target_issue_number=None,
role_kind="author",
require_lock_for_author=True,
)
self.assertTrue(result["block"])
self.assertEqual(result["blocker_kind"], wsg.BLOCKER_MISSING_ISSUE_SCOPE)
def test_branch_issue_mismatch(self):
result = wsg.assess_issue_scope_ownership(
locked_issue_number=50,
branch_name="fix/issue-99-other",
role_kind="author",
)
self.assertTrue(result["block"])
self.assertEqual(result["blocker_kind"], wsg.BLOCKER_OUT_OF_SCOPE_ISSUE)
class TestRootDiagnosticEdit(unittest.TestCase):
def test_dirty_root_source_blocked(self):
result = wsg.assess_root_source_mutation(
workspace_path=CONTROL_ROOT,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M gitea_mcp_server.py\n M tests/test_x.py\n",
role_kind="author",
)
self.assertTrue(result["block"])
self.assertEqual(result["blocker_kind"], wsg.BLOCKER_ROOT_DIAGNOSTIC_EDIT)
self.assertIn("gitea_mcp_server.py", result["dirty_source_files"])
self.assertIn("exact_next_action", result)
self.assertIn("branches/", result["exact_next_action"])
def test_isolated_worktree_same_issue_unaffected(self):
wt = f"{CONTROL_ROOT}/branches/issue-100-example"
result = wsg.assess_root_source_mutation(
workspace_path=wt,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M helper.py\n",
current_branch="fix/issue-100-example",
locked_issue_number=100,
role_kind="author",
)
self.assertFalse(result["block"])
self.assertTrue(result["under_branches"])
def test_legitimate_after_ownership_and_worktree(self):
wt = f"{CONTROL_ROOT}/branches/issue-683-workflow-guard-hardening"
composed = wsg.assess_production_mutation_guards(
workspace_path=wt,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M workflow_scope_guard.py\n",
current_branch="fix/issue-683-workflow-guard-hardening",
locked_issue_number=683,
target_issue_number=683,
role_kind="author",
require_author_lock=True,
in_test_mode=True,
)
# Force-on required for production path under pytest.
os.environ[wsg.FORCE_PRODUCTION_GUARDS_ENV] = "1"
try:
composed = wsg.assess_production_mutation_guards(
workspace_path=wt,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M workflow_scope_guard.py\n",
current_branch="fix/issue-683-workflow-guard-hardening",
locked_issue_number=683,
target_issue_number=683,
role_kind="author",
require_author_lock=True,
in_test_mode=True,
)
self.assertFalse(composed["block"])
self.assertFalse(composed.get("skipped"))
finally:
os.environ.pop(wsg.FORCE_PRODUCTION_GUARDS_ENV, None)
class TestTypedBlockerResponse(unittest.TestCase):
def test_block_response_has_stable_kind_and_next_action(self):
assessment = wsg.assess_issue_scope_ownership(
locked_issue_number=1,
target_issue_number=2,
role_kind="author",
)
resp = wsg.block_response(assessment)
self.assertFalse(resp["success"])
self.assertFalse(resp["performed"])
self.assertEqual(resp["blocker_kind"], wsg.BLOCKER_OUT_OF_SCOPE_ISSUE)
self.assertIsInstance(resp["exact_next_action"], str)
self.assertTrue(resp["exact_next_action"])
self.assertTrue(resp["reasons"])
def test_production_guard_error_roundtrip(self):
err = wsg.ProductionGuardError(
"blocked",
blocker_kind=wsg.BLOCKER_ROOT_DIAGNOSTIC_EDIT,
reasons=["dirty root"],
)
resp = wsg.block_response(err, issue_number=683)
self.assertEqual(resp["blocker_kind"], wsg.BLOCKER_ROOT_DIAGNOSTIC_EDIT)
self.assertEqual(resp["issue_number"], 683)
self.assertIn("exact_next_action", resp)
class TestDurableFailureRecording(unittest.TestCase):
def setUp(self):
wsg.clear_workflow_failure_ledger()
def tearDown(self):
wsg.clear_workflow_failure_ledger()
def test_record_before_source_mutation(self):
pending = wsg.assess_durable_failure_recorded(
require_record=True, pending_source_mutation=True
)
self.assertTrue(pending["block"])
self.assertEqual(pending["blocker_kind"], wsg.BLOCKER_UNRECORDED_FAILURE)
wsg.record_workflow_failure(
kind="transport_eof",
detail="EOF during review session (#584 cluster)",
issue_number=683,
task="comment_issue",
)
after = wsg.assess_durable_failure_recorded(
require_record=True, pending_source_mutation=True
)
self.assertFalse(after["block"])
self.assertEqual(len(wsg.workflow_failure_ledger()), 1)
class TestMonkeypatchCannotNoopFullPath(unittest.TestCase):
def tearDown(self):
os.environ.pop(wsg.FORCE_PRODUCTION_GUARDS_ENV, None)
def test_patching_branches_only_still_blocks_dirty_root_scope(self):
"""Monkeypatching branches-only must not silence root diagnostic block."""
os.environ[wsg.FORCE_PRODUCTION_GUARDS_ENV] = "1"
with patch.object(
mcp_server, "_enforce_branches_only_author_mutation", lambda *a, **k: None
):
with patch.object(
mcp_server, "_enforce_root_checkout_guard", lambda *a, **k: None
):
# Even if both legacy helpers are patched, issue-scope composition
# still sees dirty root source via assess_production_mutation_guards.
assessment = wsg.assess_production_mutation_guards(
workspace_path=CONTROL_ROOT,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M gitea_mcp_server.py\n",
role_kind="author",
in_test_mode=True,
)
self.assertTrue(assessment["block"])
self.assertEqual(
assessment["blocker_kind"], wsg.BLOCKER_ROOT_DIAGNOSTIC_EDIT
)
class TestRealEntrypointProductionGuard(unittest.TestCase):
"""Real mutation entrypoint: production guard before side effects (#683)."""
def setUp(self):
os.environ[wsg.FORCE_PRODUCTION_GUARDS_ENV] = "1"
for key in ("GITEA_AUTHOR_WORKTREE", "GITEA_ACTIVE_WORKTREE"):
os.environ.pop(key, None)
self._orig_whoami = mcp_server._preflight_whoami_called
self._orig_cap = mcp_server._preflight_capability_called
mcp_server._preflight_whoami_called = False
mcp_server._preflight_capability_called = False
mcp_server._preflight_resolved_role = None
mcp_server._preflight_resolved_task = None
def tearDown(self):
os.environ.pop(wsg.FORCE_PRODUCTION_GUARDS_ENV, None)
for key in ("GITEA_AUTHOR_WORKTREE", "GITEA_ACTIVE_WORKTREE"):
os.environ.pop(key, None)
mcp_server._preflight_whoami_called = self._orig_whoami
mcp_server._preflight_capability_called = self._orig_cap
mcp_server._preflight_resolved_role = None
mcp_server._preflight_resolved_task = None
def test_comment_issue_blocks_dirty_root_before_api(self):
api_mock = MagicMock()
with patch.object(mcp_server, "api_request", api_mock), patch.object(
mcp_server,
"_actual_profile_role",
return_value="author",
), patch.object(
mcp_server,
"_effective_workspace_role",
return_value="author",
), patch.object(
mcp_server,
"get_profile",
return_value={
"profile_name": "prgs-author",
"allowed_operations": [
"gitea.issue.comment",
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
],
"forbidden_operations": [],
},
), patch.object(
issue_lock_worktree,
"read_worktree_git_state",
side_effect=lambda path, **kw: {
"current_branch": "master",
"porcelain_status": (
" M gitea_mcp_server.py\n"
if os.path.realpath(path) == os.path.realpath(CONTROL_ROOT)
or path == CONTROL_ROOT
else ""
),
"head_sha": "a" * 40,
"base_equivalent": True,
},
), patch.object(
mcp_server,
"_resolve_namespace_mutation_context",
return_value={
"workspace_path": CONTROL_ROOT,
"canonical_repo_root": CONTROL_ROOT,
"process_project_root": CONTROL_ROOT,
"workspace_role_kind": "author",
"workspace_binding_source": "process root",
"ignored_bindings": [],
},
), patch.object(
mcp_server,
"_resolve_author_mutation_context",
return_value={
"workspace_path": CONTROL_ROOT,
"canonical_repo_root": CONTROL_ROOT,
"process_project_root": CONTROL_ROOT,
"roots_aligned": True,
},
), patch.object(
mcp_server,
"_session_locked_issue_number",
return_value=None,
):
result = mcp_server.gitea_create_issue_comment(
issue_number=683,
body="diagnostic note",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path=CONTROL_ROOT,
)
api_mock.assert_not_called()
self.assertFalse(result.get("success"))
self.assertFalse(result.get("performed"))
self.assertIn(result.get("blocker_kind"), wsg.BLOCKER_KINDS)
self.assertTrue(result.get("exact_next_action"))
self.assertTrue(result.get("reasons"))
def test_comment_issue_succeeds_structure_after_worktree_bind(self):
"""Same-issue isolated worktree is not blocked by root diagnostic path."""
wt = f"{CONTROL_ROOT}/branches/issue-683-workflow-guard-hardening"
os.environ["GITEA_AUTHOR_WORKTREE"] = wt
assessment = wsg.assess_production_mutation_guards(
workspace_path=wt,
canonical_repo_root=CONTROL_ROOT,
porcelain_status=" M workflow_scope_guard.py\n",
current_branch="fix/issue-683-workflow-guard-hardening",
locked_issue_number=683,
target_issue_number=683,
role_kind="author",
require_author_lock=True,
in_test_mode=True,
)
self.assertFalse(assessment["block"], assessment)
class TestVerifyPreflightForceOn(unittest.TestCase):
def tearDown(self):
os.environ.pop(wsg.FORCE_PRODUCTION_GUARDS_ENV, None)
for key in ("GITEA_AUTHOR_WORKTREE", "GITEA_ACTIVE_WORKTREE"):
os.environ.pop(key, None)
def test_force_on_runs_production_guards_under_pytest(self):
os.environ[wsg.FORCE_PRODUCTION_GUARDS_ENV] = "1"
called = {"root": 0, "branches": 0, "scope": 0}
def _root(*a, **k):
called["root"] += 1
def _branches(*a, **k):
called["branches"] += 1
def _scope(*a, **k):
called["scope"] += 1
with patch.object(mcp_server, "_enforce_root_checkout_guard", _root), patch.object(
mcp_server, "_enforce_branches_only_author_mutation", _branches
), patch.object(mcp_server, "_enforce_issue_scope_guard", _scope):
# No whoami/capability — purity-order skipped; production still runs.
mcp_server.verify_preflight_purity(task="comment_issue")
self.assertEqual(called["root"], 1)
self.assertEqual(called["branches"], 1)
self.assertEqual(called["scope"], 1)
def test_without_force_on_pytest_skips_production_only_for_unit_isolation(self):
called = {"root": 0}
def _root(*a, **k):
called["root"] += 1
with patch.object(mcp_server, "_enforce_root_checkout_guard", _root), patch.object(
mcp_server, "_enforce_branches_only_author_mutation", lambda *a, **k: None
), patch.object(mcp_server, "_enforce_issue_scope_guard", lambda *a, **k: None):
mcp_server.verify_preflight_purity(task="comment_issue")
self.assertEqual(called["root"], 0)
if __name__ == "__main__":
unittest.main()
+27 -6
View File
@@ -108,13 +108,24 @@ class TestIssueCommentWorkspaceGuard(unittest.TestCase):
"gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state",
side_effect=self._git_state(valid_worktree), side_effect=self._git_state(valid_worktree),
): ):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue_comment( res = srv.gitea_create_issue_comment(
issue_number=557, issue_number=557,
body="evidence comment", body="evidence comment",
remote="prgs", remote="prgs",
) )
self.assertIn("stable control checkout", str(ctx.exception)) except RuntimeError as exc:
self.assertIn("stable control checkout", str(exc))
else:
# #683 typed blocker at mutation entrypoint
self.assertFalse(res.get("success"))
self.assertFalse(res.get("performed"))
blob = " ".join(res.get("reasons") or [])
self.assertTrue(
"stable control checkout" in blob
or res.get("blocker_kind")
)
self.assertTrue(res.get("exact_next_action") or res.get("reasons"))
mock_api.assert_not_called() mock_api.assert_not_called()
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
@@ -184,14 +195,24 @@ class TestIssueCommentWorkspaceGuard(unittest.TestCase):
side_effect=self._subprocess(valid_worktree, outside_worktree), side_effect=self._subprocess(valid_worktree, outside_worktree),
): ):
with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): with patch.dict(os.environ, self.AUTHOR_ENV, clear=True):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue_comment( res = srv.gitea_create_issue_comment(
issue_number=557, issue_number=557,
body="evidence comment", body="evidence comment",
remote="prgs", remote="prgs",
worktree_path=outside_worktree, worktree_path=outside_worktree,
) )
self.assertIn("does not belong to the target repository", str(ctx.exception)) except RuntimeError as exc:
self.assertIn(
"does not belong to the target repository",
str(exc),
)
else:
self.assertFalse(res.get("success"))
blob = " ".join(res.get("reasons") or [])
self.assertIn(
"does not belong to the target repository", blob
)
mock_api.assert_not_called() mock_api.assert_not_called()
@patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH)
+9 -3
View File
@@ -1425,10 +1425,16 @@ class TestReviewPR(unittest.TestCase):
class TestDeleteBranch(unittest.TestCase): class TestDeleteBranch(unittest.TestCase):
DELETE_PROFILE = { DELETE_PROFILE = {
"profile_name": "test-deleter", "profile_name": "test-author-deleter",
"allowed_operations": ["gitea.read", "gitea.branch.delete"], "role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"forbidden_operations": [], "forbidden_operations": [],
"audit_label": "test-deleter", "audit_label": "test-author-deleter",
} }
@patch("mcp_server.get_profile", return_value=DELETE_PROFILE) @patch("mcp_server.get_profile", return_value=DELETE_PROFILE)
+176 -6
View File
@@ -92,14 +92,19 @@ class TestMigrateProfiles(unittest.TestCase):
author = prgs_gitea["identities"]["author"] author = prgs_gitea["identities"]["author"]
self.assertEqual(author["username"], "jcwalker3") self.assertEqual(author["username"], "jcwalker3")
self.assertEqual(author["auth"]["id"], "redacted-author-ref") self.assertEqual(author["auth"]["id"], "redacted-author-ref")
self.assertEqual(author["allowed_operations"], ["read", "comment"]) self.assertEqual(
self.assertEqual(author["forbidden_operations"], ["approve", "merge"]) author["allowed_operations"], ["gitea.read", "gitea.pr.comment"]
)
self.assertEqual(
author["forbidden_operations"],
["gitea.pr.approve", "gitea.pr.merge"],
)
reviewer = prgs_gitea["identities"]["reviewer"] reviewer = prgs_gitea["identities"]["reviewer"]
self.assertEqual(reviewer["role"], "reviewer") self.assertEqual(reviewer["role"], "reviewer")
self.assertEqual(reviewer["username"], "sysadmin") self.assertEqual(reviewer["username"], "sysadmin")
self.assertEqual(reviewer["auth"]["id"], "redacted-reviewer-ref") self.assertEqual(reviewer["auth"]["id"], "redacted-reviewer-ref")
self.assertIn("merge", reviewer["allowed_operations"]) self.assertIn("gitea.pr.merge", reviewer["allowed_operations"])
def test_alias_generation(self): def test_alias_generation(self):
"""Test that aliases are correctly generated to support old profile names.""" """Test that aliases are correctly generated to support old profile names."""
@@ -188,7 +193,7 @@ class TestMigrateProfiles(unittest.TestCase):
self.assertNotIn("token", stdout_output.lower()) self.assertNotIn("token", stdout_output.lower())
def test_explicit_operations_are_preserved(self): def test_explicit_operations_are_preserved(self):
"""Explicit v1 permissions must not be replaced by role defaults.""" """Explicit v1 permissions are canonicalized, not replaced by role defaults."""
v1_data = json.loads(json.dumps(self.v1_content)) v1_data = json.loads(json.dumps(self.v1_content))
v1_data["profiles"]["prgs-reviewer"]["allowed_operations"] = ["read"] v1_data["profiles"]["prgs-reviewer"]["allowed_operations"] = ["read"]
v1_data["profiles"]["prgs-reviewer"]["forbidden_operations"] = ["merge"] v1_data["profiles"]["prgs-reviewer"]["forbidden_operations"] = ["merge"]
@@ -198,8 +203,8 @@ class TestMigrateProfiles(unittest.TestCase):
v2_data["environments"]["prgs"]["services"]["gitea"] v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reviewer"] ["identities"]["reviewer"]
) )
self.assertEqual(reviewer["allowed_operations"], ["read"]) self.assertEqual(reviewer["allowed_operations"], ["gitea.read"])
self.assertEqual(reviewer["forbidden_operations"], ["merge"]) self.assertEqual(reviewer["forbidden_operations"], ["gitea.pr.merge"])
def test_inferred_role_defaults_only_when_unambiguous(self): def test_inferred_role_defaults_only_when_unambiguous(self):
"""Role defaults are allowed only for clear author/reviewer profiles.""" """Role defaults are allowed only for clear author/reviewer profiles."""
@@ -306,6 +311,171 @@ class TestMigrateProfiles(unittest.TestCase):
migrate_profiles.main() migrate_profiles.main()
self.assertEqual(cm.exception.code, 1) self.assertEqual(cm.exception.code, 1)
def test_reconciler_profile_migration(self):
"""Legacy reconciler shorthands migrate to valid canonical operations."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": [
"read",
"pr.close",
"pr.comment",
"issue.comment",
"issue.close",
"gitea.branch.delete",
],
"forbidden_operations": [
"merge",
"approve",
"review",
"pr.create",
"branch.push",
"commit",
],
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
allowed = reconciler["allowed_operations"]
forbidden = reconciler["forbidden_operations"]
# No invalid shorthand remains
for bad in ("pr.close", "pr.comment", "issue.close", "read", "merge"):
self.assertNotIn(bad, allowed)
self.assertNotIn(bad, forbidden)
for required in (
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
):
self.assertIn(required, allowed)
# Production loader accepts every allowed op
self.assertEqual(
gitea_config.normalize_operation(required), required
)
self.assertEqual(v2_data["aliases"]["prgs-reconciler"], "prgs.gitea.reconciler")
assessment = reconciler_profile.assess_reconciler_profile(allowed, forbidden)
self.assertTrue(assessment["valid"])
self.assertTrue(migrate_profiles.validate_v2_data(v2_data))
def test_reconciler_profile_defaults(self):
"""Reconciler defaults are fully canonical and loader-valid."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
self.assertEqual(
reconciler["allowed_operations"],
migrate_profiles.RECONCILER_DEFAULT_ALLOWED,
)
self.assertEqual(
reconciler["forbidden_operations"],
migrate_profiles.RECONCILER_DEFAULT_FORBIDDEN,
)
for op in reconciler["allowed_operations"]:
self.assertEqual(gitea_config.normalize_operation(op), op)
self.assertTrue(op.startswith("gitea."))
assessment = reconciler_profile.assess_reconciler_profile(
reconciler["allowed_operations"],
reconciler["forbidden_operations"],
)
self.assertTrue(assessment["valid"])
self.assertNotIn(
"gitea.branch.delete", assessment["missing_recommended_operations"]
)
def test_reconciler_migration_idempotent_canonicalize(self):
"""Second canonicalize of already-canonical ops is a no-op."""
first = migrate_profiles.canonicalize_operations(
list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED)
)
second = migrate_profiles.canonicalize_operations(first)
self.assertEqual(first, second)
self.assertEqual(first, list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED))
def test_reconciler_missing_required_fails_visibly(self):
"""Missing gitea.pr.close after migration fails closed (not silent drop)."""
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": ["read", "gitea.branch.delete"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "missing required"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_unknown_operation_fails_visibly(self):
v1_data = {
"version": 1,
"profiles": {
"prgs-author": {
"base_url": "redacted-prgs-service",
"username": "jcwalker3",
"auth": {"type": "keychain", "id": "hidden-author-ref"},
"execution_profile": "prgs-author",
"allowed_operations": ["read", "not.a.real.op"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "cannot be canonicalized"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_role_inference_author_reviewer_merger_reconciler(self):
self.assertEqual(
migrate_profiles.infer_role("prgs-author", "prgs-author"), "author"
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reviewer", "prgs-reviewer"),
"reviewer",
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reconciler", "prgs-reconciler"),
"reconciler",
)
self.assertIsNone(
migrate_profiles.infer_role("prgs-merger", "prgs-merger")
)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+7 -3
View File
@@ -76,13 +76,17 @@ class TestPreflightReadSurvival(unittest.TestCase):
self.assertIn("task mismatch", str(ctx.exception)) self.assertIn("task mismatch", str(ctx.exception))
def test_capability_consumed_after_mutation_gate(self): def test_capability_consumed_after_mutation_gate(self):
# Use reconciler/close_pr so this purity-order test does not require a
# branches/ worktree (author create_issue would hit #274/#683 guards).
# Test isolation stays explicit; production author guards remain live
# under force-on (see tests/test_issue_683_workflow_scope_guards.py).
mcp_server.record_preflight_check("whoami") mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check( mcp_server.record_preflight_check(
"capability", resolved_role="author", resolved_task="create_issue" "capability", resolved_role="reconciler", resolved_task="close_pr"
) )
mcp_server.verify_preflight_purity(task="create_issue") mcp_server.verify_preflight_purity(task="close_pr")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(RuntimeError) as ctx:
mcp_server.verify_preflight_purity(task="create_issue") mcp_server.verify_preflight_purity(task="close_pr")
self.assertIn("has not been resolved", str(ctx.exception)) self.assertIn("has not been resolved", str(ctx.exception))
def test_whoami_recovery_after_violation_clears_capability(self): def test_whoami_recovery_after_violation_clears_capability(self):
+15 -3
View File
@@ -86,9 +86,21 @@ class TestReconcilerCloseWorkspaceGuard(unittest.TestCase):
): ):
srv._preflight_resolved_role = "author" srv._preflight_resolved_role = "author"
with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT):
with self.assertRaises(RuntimeError) as ctx: try:
srv.gitea_create_issue(title="Test", body="body") res = srv.gitea_create_issue(title="Test", body="body")
self.assertIn("stable control checkout", str(ctx.exception)) except RuntimeError as exc:
self.assertIn("stable control checkout", str(exc))
else:
# #683 typed blocker at mutation entrypoint
self.assertFalse(res.get("success"))
blob = " ".join(res.get("reasons") or []) + str(
res.get("blocker_kind") or ""
)
self.assertTrue(
"stable control checkout" in blob
or "missing_issue_worktree" in blob
or "control checkout" in blob.lower()
)
if __name__ == "__main__": if __name__ == "__main__":
+36
View File
@@ -82,6 +82,42 @@ class TestReconcilerProfileModel(unittest.TestCase):
"reconciler", "reconciler",
) )
def test_branch_delete_is_recommended_for_reconciler(self):
self.assertIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_RECOMMENDED_OPERATIONS,
)
self.assertNotIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_REQUIRED_OPERATIONS,
)
def test_reconciler_with_branch_delete_stays_valid(self):
allowed = PRGS_RECONCILER_ALLOWED + ["gitea.branch.delete"]
result = reconciler_profile.assess_reconciler_profile(
allowed,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["is_reconciler_profile"])
self.assertTrue(result["valid"])
self.assertNotIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
self.assertEqual(
mcp_server._role_kind(allowed, PRGS_RECONCILER_FORBIDDEN),
"reconciler",
)
def test_reconciler_without_branch_delete_reports_missing_recommended(self):
result = reconciler_profile.assess_reconciler_profile(
PRGS_RECONCILER_ALLOWED,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["valid"])
self.assertIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+626
View File
@@ -0,0 +1,626 @@
"""Workflow scope ownership and production-guard hardening (#683).
Implements fail-closed enforcement so sessions cannot:
* mutate source/tests on the root/control checkout (including temporary
diagnostic edits) without binding an issue-backed ``branches/`` worktree;
* continue out-of-scope source work while locked to a different issue;
* disable, skip, or conceal production root/branches/porcelain guards solely
because pytest/unittest is loaded.
This module is pure assessment + small durable ledger helpers. Callers gather
live facts (lock, branch, porcelain, worktree path) and pass them in. Existing
root_checkout_guard / author_mutation_worktree assessors remain authoritative;
this module composes typed blockers with exact recovery actions.
Do **not** reintroduce the rejected #681 / ``300a4ca`` patterns:
* early-return from workspace verification under ``_preflight_in_test_mode()``
* porcelain filtering that strips ``*.py`` lines under pytest
"""
from __future__ import annotations
import os
import re
import threading
from typing import Any
import author_mutation_worktree
from reviewer_worktree import parse_dirty_tracked_files
# ── force-on / test isolation ────────────────────────────────────────────────
# When set, production root/branches/scope guards MUST run even under pytest.
# Unit tests that only need preflight-order isolation leave this unset and
# use GITEA_TEST_PORCELAIN / fixtures; real-entrypoint proof sets this to "1".
FORCE_PRODUCTION_GUARDS_ENV = "GITEA_TEST_FORCE_PRODUCTION_GUARDS"
# Existing force signals also mean "exercise production dirtiness paths".
_FORCE_DIRTY_ENV = "GITEA_TEST_FORCE_DIRTY"
_FORCE_PORCELAIN_ENV = "GITEA_TEST_PORCELAIN"
# ── typed blocker kinds ──────────────────────────────────────────────────────
BLOCKER_ROOT_DIAGNOSTIC_EDIT = "root_diagnostic_edit"
BLOCKER_MISSING_ISSUE_SCOPE = "missing_issue_scope"
BLOCKER_OUT_OF_SCOPE_ISSUE = "out_of_scope_issue"
BLOCKER_MISSING_WORKTREE = "missing_issue_worktree"
BLOCKER_UNRECORDED_FAILURE = "unrecorded_workflow_failure"
BLOCKER_PRODUCTION_GUARD = "production_guard_violation"
BLOCKER_KINDS = frozenset(
{
BLOCKER_ROOT_DIAGNOSTIC_EDIT,
BLOCKER_MISSING_ISSUE_SCOPE,
BLOCKER_OUT_OF_SCOPE_ISSUE,
BLOCKER_MISSING_WORKTREE,
BLOCKER_UNRECORDED_FAILURE,
BLOCKER_PRODUCTION_GUARD,
}
)
_NEXT_ACTIONS: dict[str, str] = {
BLOCKER_ROOT_DIAGNOSTIC_EDIT: (
"Stop editing the control/root checkout. Preserve or discard root WIP "
"durably, restore root to clean master, lock or create the owning issue, "
"bind branches/issue-<N>-*, set GITEA_AUTHOR_WORKTREE to that worktree, "
"then re-run the mutation."
),
BLOCKER_MISSING_ISSUE_SCOPE: (
"Select or create the owning Gitea issue, claim/lock it "
"(gitea_mark_issue + gitea_lock_issue), bind branches/issue-<N>-* "
"from clean master, then re-run the mutation from that worktree."
),
BLOCKER_OUT_OF_SCOPE_ISSUE: (
"Stop. The active issue lock does not own this work. Release or finish "
"the current issue lease, then select/create and lock the correct "
"owning issue, bind its branches/issue-<N>-* worktree, and re-run."
),
BLOCKER_MISSING_WORKTREE: (
"Bind an issue-backed worktree under branches/ (scripts/worktree-start "
"or git worktree add branches/issue-<N>-*), set GITEA_AUTHOR_WORKTREE / "
"worktree_path to that path, keep the control checkout clean on master, "
"then re-run the mutation."
),
BLOCKER_UNRECORDED_FAILURE: (
"Record the workflow/tool failure durably first (issue comment or "
"workflow_scope_guard.record_workflow_failure), then continue only "
"inside the owning issue-backed worktree."
),
BLOCKER_PRODUCTION_GUARD: (
"Resolve the production guard violation: clean or isolate the control "
"checkout, bind the owning issue worktree under branches/, and re-run "
"with production guards active."
),
}
_ISSUE_IN_BRANCH_RE = re.compile(r"issue-(\d+)", re.IGNORECASE)
# In-process durable failure ledger (also written via optional sink callback).
_ledger_lock = threading.Lock()
_failure_ledger: list[dict[str, Any]] = []
class ProductionGuardError(RuntimeError):
"""Fail-closed production guard with typed blocker metadata (#683)."""
def __init__(
self,
message: str,
*,
blocker_kind: str,
exact_next_action: str | None = None,
reasons: list[str] | None = None,
details: dict[str, Any] | None = None,
) -> None:
super().__init__(message)
kind = (blocker_kind or "").strip()
if kind not in BLOCKER_KINDS:
kind = BLOCKER_PRODUCTION_GUARD
self.blocker_kind = kind
self.exact_next_action = (
(exact_next_action or "").strip() or _NEXT_ACTIONS[kind]
)
self.reasons = list(reasons or [message])
self.details = dict(details or {})
def production_guards_forced() -> bool:
"""True when the explicit #683 force-on flag requests production guards."""
return (os.environ.get(FORCE_PRODUCTION_GUARDS_ENV) or "").strip().lower() in {
"1",
"true",
"yes",
"on",
}
def purity_order_forced() -> bool:
"""True when tests force preflight-order dirtiness paths (legacy flags)."""
if os.environ.get(_FORCE_DIRTY_ENV):
return True
# GITEA_TEST_PORCELAIN present (even empty) means dirtiness paths are live.
if os.environ.get(_FORCE_PORCELAIN_ENV) is not None:
return True
return False
def production_guards_active(*, in_test_mode: bool) -> bool:
"""Whether production root/branches/scope guards must execute.
Production (non-test) always active. Under pytest, active when either the
explicit #683 force-on flag or legacy dirty/porcelain force signals are
set never skip production enforcement solely because tests are running
when force-on is requested.
"""
if production_guards_forced() or purity_order_forced():
return True
return not bool(in_test_mode)
def extract_issue_number_from_branch(branch_name: str | None) -> int | None:
"""Return the first issue-N number embedded in a branch name, if any."""
text = (branch_name or "").strip()
if not text:
return None
match = _ISSUE_IN_BRANCH_RE.search(text)
if not match:
return None
try:
return int(match.group(1))
except ValueError:
return None
def is_source_or_test_path(path: str) -> bool:
"""True for tracked source/test paths that must not land as root WIP."""
p = (path or "").replace("\\", "/").lstrip("./")
if not p:
return False
if p.startswith("tests/") or "/tests/" in f"/{p}":
return True
if p.endswith((".py", ".pyi", ".toml", ".cfg", ".ini", ".sh")):
return True
if p in {"requirements.txt", "pyproject.toml", "setup.py", "setup.cfg"}:
return True
return False
def dirty_source_files(porcelain_status: str) -> list[str]:
"""Tracked dirty paths that count as source/test contamination."""
dirty = parse_dirty_tracked_files(porcelain_status or "")
return [p for p in dirty if is_source_or_test_path(p)]
def assess_issue_scope_ownership(
*,
locked_issue_number: int | None,
target_issue_number: int | None = None,
branch_name: str | None = None,
role_kind: str | None = None,
require_lock_for_author: bool = False,
) -> dict[str, Any]:
"""Fail closed when the session issue lock does not own the attempted work.
* Author sessions that require a lock fail when none is held.
* When a lock exists, the target issue (tool argument) and/or the issue
number embedded in the branch must match the locked issue.
* Reviewer/merger/reconciler roles are not issue-scope owners of author
implementation work and skip the author lock requirement.
"""
role = (role_kind or "").strip().lower()
locked = locked_issue_number
if isinstance(locked, str) and locked.isdigit():
locked = int(locked)
if locked is not None:
try:
locked = int(locked)
except (TypeError, ValueError):
locked = None
target = target_issue_number
if target is not None:
try:
target = int(target)
except (TypeError, ValueError):
target = None
branch_issue = extract_issue_number_from_branch(branch_name)
reasons: list[str] = []
blocker_kind: str | None = None
# Non-author roles do not take author issue locks for implementation.
if role in {"reviewer", "merger", "reconciler"}:
return _scope_ok(locked, target, branch_issue)
if require_lock_for_author and locked is None:
reasons.append(
"no owning issue lock is bound for this author session; "
"source/test mutation requires selecting or creating an owning issue first"
)
blocker_kind = BLOCKER_MISSING_ISSUE_SCOPE
if locked is not None and target is not None and locked != target:
reasons.append(
f"session is locked to issue #{locked} but mutation targets issue "
f"#{target}; out-of-scope until the owning issue is selected"
)
blocker_kind = BLOCKER_OUT_OF_SCOPE_ISSUE
if locked is not None and branch_issue is not None and locked != branch_issue:
reasons.append(
f"session is locked to issue #{locked} but workspace branch is for "
f"issue #{branch_issue}; bind the matching issue-backed worktree"
)
blocker_kind = BLOCKER_OUT_OF_SCOPE_ISSUE
if reasons:
kind = blocker_kind or BLOCKER_MISSING_ISSUE_SCOPE
return {
"proven": False,
"block": True,
"blocker_kind": kind,
"exact_next_action": _NEXT_ACTIONS[kind],
"reasons": reasons,
"locked_issue_number": locked,
"target_issue_number": target,
"branch_issue_number": branch_issue,
}
return _scope_ok(locked, target, branch_issue)
def assess_root_source_mutation(
*,
workspace_path: str,
canonical_repo_root: str,
porcelain_status: str,
current_branch: str | None = None,
locked_issue_number: int | None = None,
role_kind: str | None = None,
) -> dict[str, Any]:
"""Fail closed for diagnostic/source edits on the control/root checkout.
Allowed only when the active workspace is under ``branches/``. Dirty
tracked source/test files on the control checkout always block, including
temporary/diagnostic/test-only intent.
"""
role = (role_kind or "").strip().lower()
if role == "reconciler":
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
"dirty_source_files": [],
}
root = os.path.realpath(canonical_repo_root or "")
workspace = os.path.realpath(workspace_path or root or ".")
under_branches = author_mutation_worktree.is_path_under_branches(workspace, root)
dirty_src = dirty_source_files(porcelain_status)
reasons: list[str] = []
blocker_kind: str | None = None
if not under_branches and workspace == root and dirty_src:
# Root workspace with source dirtiness is unattributed root WIP.
# (Clean-root author binding is enforced by branches-only #274.)
reasons.append(
"control/root checkout has tracked source or test edits "
f"(dirty files: {', '.join(dirty_src)}); diagnostic or temporary "
"edits on the root checkout are forbidden"
)
blocker_kind = BLOCKER_ROOT_DIAGNOSTIC_EDIT
if (
not under_branches
and workspace == root
and not dirty_src
and role == "author"
):
# Explicit missing-worktree signal for force-on author entrypoints.
reasons.append(
"author source/test mutation from the stable control checkout is "
"forbidden; bind an issue-backed worktree under branches/ first"
)
blocker_kind = BLOCKER_MISSING_WORKTREE
if reasons:
kind = blocker_kind or BLOCKER_ROOT_DIAGNOSTIC_EDIT
return {
"proven": False,
"block": True,
"blocker_kind": kind,
"exact_next_action": _NEXT_ACTIONS[kind],
"reasons": reasons,
"dirty_source_files": dirty_src,
"workspace_path": workspace,
"canonical_repo_root": root,
"under_branches": under_branches,
"locked_issue_number": locked_issue_number,
}
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
"dirty_source_files": dirty_src,
"workspace_path": workspace,
"canonical_repo_root": root,
"under_branches": under_branches,
"locked_issue_number": locked_issue_number,
}
def assess_production_mutation_guards(
*,
workspace_path: str,
canonical_repo_root: str,
porcelain_status: str,
current_branch: str | None = None,
locked_issue_number: int | None = None,
target_issue_number: int | None = None,
role_kind: str | None = None,
require_author_lock: bool = False,
in_test_mode: bool = False,
) -> dict[str, Any]:
"""Compose root + scope production guards when they must be active (#683)."""
if not production_guards_active(in_test_mode=in_test_mode):
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
"skipped": True,
"skip_reason": "production guards not active (test isolation without force-on)",
}
root_assess = assess_root_source_mutation(
workspace_path=workspace_path,
canonical_repo_root=canonical_repo_root,
porcelain_status=porcelain_status,
current_branch=current_branch,
locked_issue_number=locked_issue_number,
role_kind=role_kind,
)
if root_assess["block"]:
return {**root_assess, "skipped": False}
scope_assess = assess_issue_scope_ownership(
locked_issue_number=locked_issue_number,
target_issue_number=target_issue_number,
branch_name=current_branch,
role_kind=role_kind,
require_lock_for_author=require_author_lock,
)
if scope_assess["block"]:
return {**scope_assess, "skipped": False}
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
"skipped": False,
"root": root_assess,
"scope": scope_assess,
}
def raise_if_blocked(assessment: dict[str, Any]) -> None:
"""Raise :class:`ProductionGuardError` when *assessment* blocks."""
if not assessment or not assessment.get("block"):
return
kind = assessment.get("blocker_kind") or BLOCKER_PRODUCTION_GUARD
reasons = list(assessment.get("reasons") or ["production guard violation"])
message = (
f"Workflow scope guard (#683) [{kind}]: {'; '.join(reasons)}. "
f"exact_next_action: {assessment.get('exact_next_action') or _NEXT_ACTIONS.get(kind, '')}"
)
raise ProductionGuardError(
message,
blocker_kind=kind,
exact_next_action=assessment.get("exact_next_action"),
reasons=reasons,
details={
k: v
for k, v in assessment.items()
if k
not in {
"proven",
"block",
"blocker_kind",
"exact_next_action",
"reasons",
}
},
)
def block_response(
assessment: dict[str, Any] | ProductionGuardError | None = None,
*,
blocker_kind: str | None = None,
reasons: list[str] | None = None,
exact_next_action: str | None = None,
**extra: Any,
) -> dict[str, Any]:
"""Structured fail-closed tool response with typed blocker fields."""
if isinstance(assessment, ProductionGuardError):
kind = assessment.blocker_kind
reason_list = list(assessment.reasons)
next_action = assessment.exact_next_action
extra = {**assessment.details, **extra}
elif isinstance(assessment, dict) and assessment.get("block"):
kind = assessment.get("blocker_kind") or BLOCKER_PRODUCTION_GUARD
reason_list = list(assessment.get("reasons") or [])
next_action = assessment.get("exact_next_action") or _NEXT_ACTIONS.get(
kind, _NEXT_ACTIONS[BLOCKER_PRODUCTION_GUARD]
)
else:
kind = (blocker_kind or BLOCKER_PRODUCTION_GUARD).strip()
if kind not in BLOCKER_KINDS:
kind = BLOCKER_PRODUCTION_GUARD
reason_list = list(reasons or ["production guard violation"])
next_action = exact_next_action or _NEXT_ACTIONS[kind]
if kind not in BLOCKER_KINDS:
kind = BLOCKER_PRODUCTION_GUARD
if not reason_list:
reason_list = ["production guard violation"]
next_action = (next_action or "").strip() or _NEXT_ACTIONS[kind]
out: dict[str, Any] = {
"success": False,
"performed": False,
"blocker_kind": kind,
"exact_next_action": next_action,
"reasons": reason_list,
}
for key, value in extra.items():
if key not in out and value is not None:
out[key] = value
return out
def format_production_guard_error(assessment: dict[str, Any]) -> str:
"""Single RuntimeError string carrying kind + exact next action."""
kind = assessment.get("blocker_kind") or BLOCKER_PRODUCTION_GUARD
reasons = "; ".join(assessment.get("reasons") or ["production guard violation"])
next_action = assessment.get("exact_next_action") or _NEXT_ACTIONS.get(
kind, _NEXT_ACTIONS[BLOCKER_PRODUCTION_GUARD]
)
return (
f"Workflow scope guard (#683) [{kind}]: {reasons}. "
f"exact_next_action: {next_action}"
)
# ── durable failure recording ────────────────────────────────────────────────
def record_workflow_failure(
*,
kind: str,
detail: str,
issue_number: int | None = None,
task: str | None = None,
sink: Any | None = None,
) -> dict[str, Any]:
"""Record a workflow/tool failure before source edits continue (#683 AC8).
*sink* may be a callable ``sink(record)`` (e.g. tests) or omitted for the
in-process ledger only. Returns the durable record.
"""
record = {
"kind": (kind or "workflow_failure").strip() or "workflow_failure",
"detail": (detail or "").strip(),
"issue_number": issue_number,
"task": task,
"pid": os.getpid(),
}
with _ledger_lock:
_failure_ledger.append(dict(record))
if callable(sink):
sink(record)
return record
def clear_workflow_failure_ledger() -> None:
"""Test helper: reset the in-process failure ledger."""
with _ledger_lock:
_failure_ledger.clear()
def workflow_failure_ledger() -> list[dict[str, Any]]:
"""Copy of durable in-process failure records."""
with _ledger_lock:
return [dict(r) for r in _failure_ledger]
def assess_durable_failure_recorded(
*,
require_record: bool,
pending_source_mutation: bool,
) -> dict[str, Any]:
"""Block source mutation when a workflow failure was not recorded first."""
if not require_record or not pending_source_mutation:
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
}
with _ledger_lock:
has_record = bool(_failure_ledger)
if has_record:
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
}
return {
"proven": False,
"block": True,
"blocker_kind": BLOCKER_UNRECORDED_FAILURE,
"exact_next_action": _NEXT_ACTIONS[BLOCKER_UNRECORDED_FAILURE],
"reasons": [
"workflow/tool failure triggered a need for source changes but no "
"durable failure record exists yet"
],
}
def porcelain_preserves_python_paths(porcelain_status: str) -> bool:
"""Regression helper: dirty ``*.py`` lines must remain visible (#683)."""
text = porcelain_status or ""
for line in text.splitlines():
stripped = line.strip()
if stripped.endswith(".py") or ".py " in stripped or stripped.endswith(".py"):
# Any py path present proves no silent strip of all *.py lines.
if " M " in f" {stripped}" or stripped[:1] in "MADRCTU" or len(line) >= 4:
return True
# Empty porcelain is fine; integrity means we did not strip when present.
return ".py" not in text
def assert_no_pytest_porcelain_filter(source_text: str) -> list[str]:
"""Static check: production reader must not strip ``*.py`` under pytest."""
findings: list[str] = []
lowered = source_text or ""
if "endswith(\".py\")" in lowered or "endswith('.py')" in lowered:
if "pytest" in lowered and "porcelain" in lowered.lower():
findings.append(
"production porcelain reader must not filter *.py under pytest "
"(rejected 300a4ca pattern)"
)
if "if \"pytest\" in sys.modules" in lowered and "porcelain" in lowered.lower():
if ".py" in lowered and ("join" in lowered or "endswith" in lowered):
findings.append(
"test-mode porcelain filtering of source files is forbidden (#683)"
)
return findings
def _scope_ok(
locked: int | None,
target: int | None,
branch_issue: int | None,
) -> dict[str, Any]:
return {
"proven": True,
"block": False,
"blocker_kind": None,
"exact_next_action": "proceed",
"reasons": [],
"locked_issue_number": locked,
"target_issue_number": target,
"branch_issue_number": branch_issue,
}