Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e6a0603fc3 |
@@ -7,11 +7,8 @@ project's ``branches/`` directory, never from the stable control checkout.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import os
|
import os
|
||||||
import subprocess
|
|
||||||
|
|
||||||
BASE_BRANCHES = frozenset({"master", "main", "dev"})
|
BASE_BRANCHES = frozenset({"master", "main", "dev"})
|
||||||
ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE"
|
|
||||||
AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE"
|
|
||||||
|
|
||||||
|
|
||||||
def _normalize_path(path: str) -> str:
|
def _normalize_path(path: str) -> str:
|
||||||
@@ -51,130 +48,6 @@ def resolve_mutation_workspace(
|
|||||||
return os.path.realpath(project_root)
|
return os.path.realpath(project_root)
|
||||||
|
|
||||||
|
|
||||||
def _realpath_git_common_dir(workspace_path: str, common_dir: str) -> str:
|
|
||||||
"""Resolve ``git rev-parse --git-common-dir`` relative to *workspace_path*."""
|
|
||||||
raw = (common_dir or "").strip()
|
|
||||||
if not raw:
|
|
||||||
return raw
|
|
||||||
if os.path.isabs(raw):
|
|
||||||
return os.path.realpath(raw)
|
|
||||||
return os.path.realpath(os.path.join(workspace_path, raw))
|
|
||||||
|
|
||||||
|
|
||||||
def resolve_canonical_repo_root(workspace_path: str, fallback_project_root: str) -> str:
|
|
||||||
"""Return the stable repository root for *workspace_path* via git metadata (#460)."""
|
|
||||||
path = (workspace_path or "").strip()
|
|
||||||
fallback = os.path.realpath(fallback_project_root)
|
|
||||||
if not path:
|
|
||||||
return fallback
|
|
||||||
try:
|
|
||||||
res = subprocess.run(
|
|
||||||
["git", "-C", path, "rev-parse", "--git-common-dir"],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
check=True,
|
|
||||||
)
|
|
||||||
common = _realpath_git_common_dir(path, res.stdout)
|
|
||||||
except Exception:
|
|
||||||
return fallback
|
|
||||||
if common.endswith(f"{os.sep}.git"):
|
|
||||||
return os.path.dirname(common)
|
|
||||||
if os.path.basename(common) == ".git":
|
|
||||||
return os.path.dirname(common)
|
|
||||||
return fallback
|
|
||||||
|
|
||||||
|
|
||||||
def resolve_author_mutation_context(
|
|
||||||
worktree_path: str | None,
|
|
||||||
process_project_root: str,
|
|
||||||
*,
|
|
||||||
active_worktree_env: str | None = None,
|
|
||||||
author_worktree_env: str | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Shared workspace resolution for runtime_context and mutation guards (#460)."""
|
|
||||||
workspace = resolve_mutation_workspace(
|
|
||||||
worktree_path,
|
|
||||||
process_project_root,
|
|
||||||
active_worktree_env=active_worktree_env,
|
|
||||||
author_worktree_env=author_worktree_env,
|
|
||||||
)
|
|
||||||
process_root = os.path.realpath(process_project_root)
|
|
||||||
# Canonical repository identity comes from the MCP process checkout (#460),
|
|
||||||
# not from the declared task workspace being validated.
|
|
||||||
canonical_root = resolve_canonical_repo_root(process_root, process_root)
|
|
||||||
return {
|
|
||||||
"workspace_path": workspace,
|
|
||||||
"process_project_root": process_root,
|
|
||||||
"canonical_repo_root": canonical_root,
|
|
||||||
"roots_aligned": canonical_root == process_root,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def assess_workspace_repo_membership(
|
|
||||||
*,
|
|
||||||
workspace_path: str,
|
|
||||||
canonical_repo_root: str,
|
|
||||||
) -> dict:
|
|
||||||
"""Fail closed when *workspace_path* is not a git worktree of *canonical_repo_root*."""
|
|
||||||
workspace = os.path.realpath(workspace_path)
|
|
||||||
root = os.path.realpath(canonical_repo_root)
|
|
||||||
reasons: list[str] = []
|
|
||||||
|
|
||||||
if not os.path.exists(workspace):
|
|
||||||
reasons.append(f"worktree path '{workspace}' does not exist")
|
|
||||||
return _membership_assessment(False, reasons, workspace, root, None)
|
|
||||||
|
|
||||||
if not os.path.isdir(workspace):
|
|
||||||
reasons.append(f"worktree path '{workspace}' is not a directory")
|
|
||||||
return _membership_assessment(False, reasons, workspace, root, None)
|
|
||||||
|
|
||||||
try:
|
|
||||||
res = subprocess.run(
|
|
||||||
["git", "-C", workspace, "rev-parse", "--git-common-dir"],
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
check=True,
|
|
||||||
)
|
|
||||||
common_dir = _realpath_git_common_dir(workspace, res.stdout)
|
|
||||||
except Exception:
|
|
||||||
reasons.append(f"worktree '{workspace}' is not a valid git repository")
|
|
||||||
return _membership_assessment(False, reasons, workspace, root, None)
|
|
||||||
|
|
||||||
expected_dir = os.path.realpath(os.path.join(root, ".git"))
|
|
||||||
if common_dir != expected_dir:
|
|
||||||
reasons.append(
|
|
||||||
f"worktree '{workspace}' does not belong to the target repository '{root}'"
|
|
||||||
)
|
|
||||||
return _membership_assessment(not reasons, reasons, workspace, root, common_dir)
|
|
||||||
|
|
||||||
|
|
||||||
def _membership_assessment(
|
|
||||||
proven: bool,
|
|
||||||
reasons: list[str],
|
|
||||||
workspace: str,
|
|
||||||
root: str,
|
|
||||||
common_dir: str | None,
|
|
||||||
) -> dict:
|
|
||||||
return {
|
|
||||||
"proven": proven,
|
|
||||||
"block": not proven,
|
|
||||||
"reasons": reasons,
|
|
||||||
"workspace_path": workspace,
|
|
||||||
"canonical_repo_root": root,
|
|
||||||
"git_common_dir": common_dir,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def format_workspace_repo_membership_error(assessment: dict) -> str:
|
|
||||||
workspace = assessment.get("workspace_path") or "(unknown)"
|
|
||||||
root = assessment.get("canonical_repo_root") or "(unknown)"
|
|
||||||
reasons = "; ".join(assessment.get("reasons") or ["unknown repository membership violation"])
|
|
||||||
return (
|
|
||||||
f"Branches-only mutation guard (#274): {reasons} (fail closed). "
|
|
||||||
f"canonical repository root: {root}; workspace: {workspace}."
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def assess_author_mutation_worktree(
|
def assess_author_mutation_worktree(
|
||||||
*,
|
*,
|
||||||
workspace_path: str,
|
workspace_path: str,
|
||||||
|
|||||||
@@ -274,39 +274,12 @@ is proven abandoned and the takeover is recorded.
|
|||||||
|
|
||||||
Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author
|
Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author
|
||||||
mutations), `status:in-progress`, and claim comments. `gitea_lock_issue`
|
mutations), `status:in-progress`, and claim comments. `gitea_lock_issue`
|
||||||
records an `author_issue_work` lease in a keyed lock file under
|
records an `author_issue_work` lease in the issue-lock payload with issue
|
||||||
`GITEA_ISSUE_LOCK_DIR` (default `~/.cache/gitea-tools/issue-locks`), one file
|
number, optional PR number, branch, worktree path, claimant identity/profile,
|
||||||
per `remote` + `org` + `repo` + `issue_number`. The current MCP session binds
|
created timestamp, expiry timestamp, and last heartbeat timestamp. An active
|
||||||
its active lock through a per-process pointer so concurrent repos/issues never
|
same-issue/same-operation lease blocks duplicate work. An expired lease still
|
||||||
share one overwrite-prone slot (#443).
|
blocks takeover until a recovery review records why the prior work is abandoned,
|
||||||
|
completed, or unsafe to continue.
|
||||||
Each lock payload includes issue number, optional PR number, branch, worktree
|
|
||||||
path, claimant identity/profile, created timestamp, expiry timestamp, and last
|
|
||||||
heartbeat timestamp. An active same-issue/same-operation lease blocks duplicate
|
|
||||||
work. An expired lease still blocks takeover until a recovery review records why
|
|
||||||
the prior work is abandoned, completed, or unsafe to continue.
|
|
||||||
|
|
||||||
**Do not manually seed `/tmp/gitea_issue_lock.json` or any lock file as a normal
|
|
||||||
recovery path.** That global slot is deprecated and can clobber unrelated live
|
|
||||||
leases (#438). After an MCP restart, call `gitea_lock_issue` again — own-branch
|
|
||||||
adoption rebinds the session when the issue's exact branch already exists (#442).
|
|
||||||
`gitea_create_pr` resolves the durable keyed lock by session pointer or by
|
|
||||||
matching `head` branch without unsafe manual seeding.
|
|
||||||
|
|
||||||
**Issue-lock recovery (#447):** Do not manually seed, restore, or delete
|
|
||||||
`/tmp/gitea_issue_lock.json` as a normal recovery path. That file is global
|
|
||||||
shared state and manual writes can clobber another session's live lease. Use
|
|
||||||
`sanctioned recovery` instead:
|
|
||||||
|
|
||||||
1. `gitea_lock_issue` on a clean `branches/` worktree (normal path).
|
|
||||||
2. Own-branch adoption via #442 when the issue's exact branch is already pushed.
|
|
||||||
3. Operator override only when explicitly authorized — record
|
|
||||||
`External-state mutations` and `operator override proof` in the final report.
|
|
||||||
|
|
||||||
`gitea_create_pr` rejects lock files that lack sanctioned `lock_provenance`
|
|
||||||
metadata. Final-report validation blocks handoffs that hide lock read/write/delete
|
|
||||||
under `External-state mutations: none` or mix author PR creation with reviewer
|
|
||||||
approval in one run. See also #438 (global lock redesign).
|
|
||||||
|
|
||||||
Remote branches matching the issue number are also treated as active work unless
|
Remote branches matching the issue number are also treated as active work unless
|
||||||
the recovery review proves the branch is abandoned or superseded. Never delete
|
the recovery review proves the branch is abandoned or superseded. Never delete
|
||||||
|
|||||||
@@ -11,7 +11,6 @@ import inspect
|
|||||||
import re
|
import re
|
||||||
from typing import Any, Callable
|
from typing import Any, Callable
|
||||||
|
|
||||||
import issue_lock_provenance
|
|
||||||
from review_proofs import (
|
from review_proofs import (
|
||||||
HANDOFF_HEADING,
|
HANDOFF_HEADING,
|
||||||
assess_controller_handoff,
|
assess_controller_handoff,
|
||||||
@@ -871,54 +870,6 @@ def _rule_reviewer_mutation_ledger(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _rule_shared_issue_lock_external_state(report_text: str) -> list[dict[str, str]]:
|
|
||||||
result = issue_lock_provenance.assess_issue_lock_external_state_report(report_text)
|
|
||||||
if result.get("proven"):
|
|
||||||
return []
|
|
||||||
return _findings_from_reasons(
|
|
||||||
"shared.issue_lock_external_state",
|
|
||||||
result.get("reasons") or [],
|
|
||||||
field="External-state mutations",
|
|
||||||
severity="block",
|
|
||||||
safe_next_action=(
|
|
||||||
"disclose gitea_issue_lock.json read/write/delete under "
|
|
||||||
"External-state mutations; never claim none after lock seeding"
|
|
||||||
),
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str]]:
|
|
||||||
result = issue_lock_provenance.assess_manual_lock_pr_without_override(report_text)
|
|
||||||
if result.get("proven"):
|
|
||||||
return []
|
|
||||||
return _findings_from_reasons(
|
|
||||||
"shared.manual_lock_pr_override",
|
|
||||||
result.get("reasons") or [],
|
|
||||||
field="External-state mutations",
|
|
||||||
severity="block",
|
|
||||||
safe_next_action=(
|
|
||||||
"use gitea_lock_issue or #442 adoption instead of manual lock seeding; "
|
|
||||||
"if operator override was authorized, cite override proof"
|
|
||||||
),
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _rule_shared_author_reviewer_same_run(report_text: str) -> list[dict[str, str]]:
|
|
||||||
result = issue_lock_provenance.assess_author_reviewer_same_run_report(report_text)
|
|
||||||
if result.get("proven"):
|
|
||||||
return []
|
|
||||||
return _findings_from_reasons(
|
|
||||||
"shared.author_reviewer_same_run",
|
|
||||||
result.get("reasons") or [],
|
|
||||||
field="Review mutations",
|
|
||||||
severity="block",
|
|
||||||
safe_next_action=(
|
|
||||||
"split author PR creation and reviewer approval across separate "
|
|
||||||
"sessions and handoffs"
|
|
||||||
),
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _rule_reviewer_review_mutation(
|
def _rule_reviewer_review_mutation(
|
||||||
report_text: str,
|
report_text: str,
|
||||||
*,
|
*,
|
||||||
@@ -938,17 +889,10 @@ def _rule_reviewer_review_mutation(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
_SHARED_ISSUE_LOCK_RULES = (
|
|
||||||
_rule_shared_issue_lock_external_state,
|
|
||||||
_rule_shared_manual_lock_pr_override,
|
|
||||||
_rule_shared_author_reviewer_same_run,
|
|
||||||
)
|
|
||||||
|
|
||||||
_RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
_RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
||||||
"review_pr": [
|
"review_pr": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
_rule_reviewer_legacy_workspace_mutations,
|
_rule_reviewer_legacy_workspace_mutations,
|
||||||
_rule_reviewer_vague_mutations_none,
|
_rule_reviewer_vague_mutations_none,
|
||||||
_rule_reviewer_mutation_categories,
|
_rule_reviewer_mutation_categories,
|
||||||
@@ -969,7 +913,6 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
|||||||
"reconcile_already_landed": [
|
"reconcile_already_landed": [
|
||||||
_rule_reconcile_controller_handoff,
|
_rule_reconcile_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
_rule_reconcile_stale_author_fields,
|
_rule_reconcile_stale_author_fields,
|
||||||
_rule_reconcile_eligible_reviewed,
|
_rule_reconcile_eligible_reviewed,
|
||||||
_rule_reconcile_linked_issue_live,
|
_rule_reconcile_linked_issue_live,
|
||||||
@@ -981,30 +924,25 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
|
|||||||
"author_issue": [
|
"author_issue": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
_rule_reviewer_vague_mutations_none,
|
_rule_reviewer_vague_mutations_none,
|
||||||
],
|
],
|
||||||
"work_issue": [
|
"work_issue": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
_rule_reviewer_vague_mutations_none,
|
_rule_reviewer_vague_mutations_none,
|
||||||
],
|
],
|
||||||
"issue_filing": [
|
"issue_filing": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
],
|
],
|
||||||
"inventory": [
|
"inventory": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
_rule_reconcile_pagination_proof,
|
_rule_reconcile_pagination_proof,
|
||||||
],
|
],
|
||||||
"issue_selection": [
|
"issue_selection": [
|
||||||
_rule_shared_controller_handoff,
|
_rule_shared_controller_handoff,
|
||||||
_rule_shared_email_disclosure,
|
_rule_shared_email_disclosure,
|
||||||
*_SHARED_ISSUE_LOCK_RULES,
|
|
||||||
],
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+116
-638
@@ -190,22 +190,14 @@ def _ensure_process_start_porcelain() -> str:
|
|||||||
|
|
||||||
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
|
def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str:
|
||||||
"""Resolve the workspace root inspected by pre-flight guards."""
|
"""Resolve the workspace root inspected by pre-flight guards."""
|
||||||
return author_mutation_worktree.resolve_mutation_workspace(
|
path = (worktree_path or "").strip()
|
||||||
worktree_path,
|
if not path:
|
||||||
PROJECT_ROOT,
|
path = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip()
|
||||||
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
|
if not path:
|
||||||
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
|
path = (os.environ.get(AUTHOR_WORKTREE_ENV) or "").strip()
|
||||||
)
|
if not path:
|
||||||
|
path = PROJECT_ROOT
|
||||||
|
return os.path.realpath(os.path.abspath(path))
|
||||||
def _resolve_author_mutation_context(worktree_path: str | None = None) -> dict:
|
|
||||||
"""Canonical workspace + repository root for runtime_context and guards (#460)."""
|
|
||||||
return author_mutation_worktree.resolve_author_mutation_context(
|
|
||||||
worktree_path,
|
|
||||||
PROJECT_ROOT,
|
|
||||||
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
|
|
||||||
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _get_git_root(path: str) -> str | None:
|
def _get_git_root(path: str) -> str | None:
|
||||||
@@ -275,31 +267,21 @@ def _format_preflight_files(files: list[str]) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def _preflight_workspace_details(worktree_path: str | None, dirty_files: list[str]) -> dict:
|
def _preflight_workspace_details(worktree_path: str | None, dirty_files: list[str]) -> dict:
|
||||||
ctx = _resolve_author_mutation_context(worktree_path)
|
workspace = _resolve_preflight_workspace_path(worktree_path)
|
||||||
workspace = ctx["workspace_path"]
|
|
||||||
inspected_root = _get_git_root(workspace)
|
inspected_root = _get_git_root(workspace)
|
||||||
process_root = ctx["process_project_root"]
|
control_root = os.path.realpath(PROJECT_ROOT)
|
||||||
canonical_root = ctx["canonical_repo_root"]
|
|
||||||
active_root = os.path.realpath(inspected_root or workspace)
|
active_root = os.path.realpath(inspected_root or workspace)
|
||||||
if active_root == canonical_root:
|
if active_root == control_root:
|
||||||
dirty_scope = "control checkout"
|
dirty_scope = "control checkout"
|
||||||
else:
|
else:
|
||||||
dirty_scope = "active task workspace"
|
dirty_scope = "active task workspace"
|
||||||
details = {
|
return {
|
||||||
"mcp_server_process_root": process_root,
|
"mcp_server_process_root": control_root,
|
||||||
"canonical_repository_root": canonical_root,
|
|
||||||
"active_task_workspace_root": active_root,
|
"active_task_workspace_root": active_root,
|
||||||
"inspected_git_root": inspected_root,
|
"inspected_git_root": inspected_root,
|
||||||
"dirty_files": list(dirty_files),
|
"dirty_files": list(dirty_files),
|
||||||
"dirty_scope": dirty_scope,
|
"dirty_scope": dirty_scope,
|
||||||
"workspace_roots_aligned": ctx["roots_aligned"],
|
|
||||||
}
|
}
|
||||||
if not ctx["roots_aligned"]:
|
|
||||||
details["workspace_root_mismatch"] = (
|
|
||||||
"runtime_context and mutation guard use canonical repository root "
|
|
||||||
f"'{canonical_root}' instead of MCP process root '{process_root}'"
|
|
||||||
)
|
|
||||||
return details
|
|
||||||
|
|
||||||
|
|
||||||
def _format_preflight_workspace_details(details: dict) -> str:
|
def _format_preflight_workspace_details(details: dict) -> str:
|
||||||
@@ -420,12 +402,16 @@ def _enforce_branches_only_author_mutation(worktree_path: str | None = None) ->
|
|||||||
"""#274: author mutations must run from a branches/ session worktree."""
|
"""#274: author mutations must run from a branches/ session worktree."""
|
||||||
if _preflight_resolved_role == "reviewer":
|
if _preflight_resolved_role == "reviewer":
|
||||||
return
|
return
|
||||||
ctx = _resolve_author_mutation_context(worktree_path)
|
workspace = author_mutation_worktree.resolve_mutation_workspace(
|
||||||
workspace = ctx["workspace_path"]
|
worktree_path,
|
||||||
|
PROJECT_ROOT,
|
||||||
|
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
|
||||||
|
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
|
||||||
|
)
|
||||||
git_state = issue_lock_worktree.read_worktree_git_state(workspace)
|
git_state = issue_lock_worktree.read_worktree_git_state(workspace)
|
||||||
assessment = author_mutation_worktree.assess_author_mutation_worktree(
|
assessment = author_mutation_worktree.assess_author_mutation_worktree(
|
||||||
workspace_path=workspace,
|
workspace_path=workspace,
|
||||||
project_root=ctx["canonical_repo_root"],
|
project_root=PROJECT_ROOT,
|
||||||
current_branch=git_state.get("current_branch"),
|
current_branch=git_state.get("current_branch"),
|
||||||
)
|
)
|
||||||
if assessment["block"]:
|
if assessment["block"]:
|
||||||
@@ -454,23 +440,43 @@ def verify_preflight_purity(remote: str | None = None, worktree_path: str | None
|
|||||||
"Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)"
|
"Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)"
|
||||||
)
|
)
|
||||||
|
|
||||||
ctx = _resolve_author_mutation_context(worktree_path)
|
workspace = author_mutation_worktree.resolve_mutation_workspace(
|
||||||
workspace = ctx["workspace_path"]
|
worktree_path,
|
||||||
canonical_root = ctx["canonical_repo_root"]
|
PROJECT_ROOT,
|
||||||
process_root = ctx["process_project_root"]
|
active_worktree_env=os.environ.get(ACTIVE_WORKTREE_ENV),
|
||||||
|
author_worktree_env=os.environ.get(AUTHOR_WORKTREE_ENV),
|
||||||
|
)
|
||||||
real_workspace = os.path.realpath(workspace)
|
real_workspace = os.path.realpath(workspace)
|
||||||
|
real_root = os.path.realpath(PROJECT_ROOT)
|
||||||
|
|
||||||
if real_workspace != process_root:
|
if real_workspace != real_root:
|
||||||
if not _preflight_in_test_mode():
|
if not _preflight_in_test_mode():
|
||||||
membership = author_mutation_worktree.assess_workspace_repo_membership(
|
if not os.path.exists(real_workspace):
|
||||||
workspace_path=workspace,
|
|
||||||
canonical_repo_root=canonical_root,
|
|
||||||
)
|
|
||||||
if membership["block"]:
|
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
author_mutation_worktree.format_workspace_repo_membership_error(
|
f"Branches-only mutation guard (#274): worktree path '{workspace}' does not exist (fail closed)"
|
||||||
membership
|
|
||||||
)
|
)
|
||||||
|
if not os.path.isdir(real_workspace):
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Branches-only mutation guard (#274): worktree path '{workspace}' is not a directory (fail closed)"
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
res = subprocess.run(
|
||||||
|
["git", "-C", real_workspace, "rev-parse", "--git-common-dir"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=True,
|
||||||
|
)
|
||||||
|
common_dir = os.path.realpath(res.stdout.strip())
|
||||||
|
expected_dir = os.path.realpath(os.path.join(real_root, ".git"))
|
||||||
|
if common_dir != expected_dir:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Branches-only mutation guard (#274): worktree '{workspace}' does not belong to the target repository '{PROJECT_ROOT}' (fail closed)"
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
if isinstance(e, RuntimeError):
|
||||||
|
raise e
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Branches-only mutation guard (#274): worktree '{workspace}' is not a valid git repository (fail closed)"
|
||||||
)
|
)
|
||||||
|
|
||||||
dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace)))
|
dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(workspace)))
|
||||||
@@ -532,14 +538,9 @@ import task_capability_map # noqa: E402
|
|||||||
import review_proofs # noqa: E402
|
import review_proofs # noqa: E402
|
||||||
import agent_temp_artifacts
|
import agent_temp_artifacts
|
||||||
import issue_lock_worktree # noqa: E402
|
import issue_lock_worktree # noqa: E402
|
||||||
import issue_lock_provenance # noqa: E402
|
|
||||||
import issue_lock_store # noqa: E402
|
|
||||||
import issue_lock_adoption # 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 issue_claim_heartbeat # noqa: E402
|
import issue_claim_heartbeat # noqa: E402
|
||||||
import issue_work_duplicate_gate # noqa: E402
|
|
||||||
import reviewer_pr_lease # noqa: E402
|
|
||||||
import merged_cleanup_reconcile # noqa: E402
|
import merged_cleanup_reconcile # noqa: E402
|
||||||
import reconciler_profile # noqa: E402
|
import reconciler_profile # noqa: E402
|
||||||
import reconciliation_workflow # noqa: E402
|
import reconciliation_workflow # noqa: E402
|
||||||
@@ -547,9 +548,8 @@ import review_merge_state_machine # noqa: E402
|
|||||||
import native_mcp_preference # noqa: E402
|
import native_mcp_preference # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
# Keyed issue-lock storage (#443): per remote/org/repo/issue files under
|
# Fail-closed exact-issue-lock file (#204): written by gitea_lock_issue,
|
||||||
# GITEA_ISSUE_LOCK_DIR, bound to the current MCP session via a per-PID pointer.
|
# consumed by gitea_create_pr and scripts/worktree-start.
|
||||||
# Legacy global path retained only for test/doc references — do not seed manually.
|
|
||||||
ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
|
ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
|
||||||
WORK_LEASE_TTL_HOURS = 4
|
WORK_LEASE_TTL_HOURS = 4
|
||||||
AUTHOR_ISSUE_WORK_LEASE = "author_issue_work"
|
AUTHOR_ISSUE_WORK_LEASE = "author_issue_work"
|
||||||
@@ -581,59 +581,15 @@ def _parse_work_lease_timestamp(value: str | None) -> datetime | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def _load_existing_issue_lock(
|
def _load_existing_issue_lock() -> dict | None:
|
||||||
*,
|
if not os.path.exists(ISSUE_LOCK_FILE):
|
||||||
remote: str | None = None,
|
return None
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
issue_number: int | None = None,
|
|
||||||
) -> dict | None:
|
|
||||||
if remote and org and repo and issue_number is not None:
|
|
||||||
return issue_lock_store.load_issue_lock(
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
issue_number=issue_number,
|
|
||||||
)
|
|
||||||
return issue_lock_store.read_session_issue_lock()
|
|
||||||
|
|
||||||
|
|
||||||
def _resolve_issue_lock_for_pr(
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
org: str,
|
|
||||||
repo: str,
|
|
||||||
head: str,
|
|
||||||
) -> dict:
|
|
||||||
lock_data = issue_lock_store.read_session_issue_lock()
|
|
||||||
if not lock_data:
|
|
||||||
lock_data = issue_lock_store.find_lock_for_branch(
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
branch_name=head,
|
|
||||||
)
|
|
||||||
if not lock_data:
|
|
||||||
raise RuntimeError(
|
|
||||||
"Issue lock is missing (fail closed). Call gitea_lock_issue first."
|
|
||||||
)
|
|
||||||
return lock_data
|
|
||||||
|
|
||||||
|
|
||||||
def _save_issue_lock(data: dict) -> str:
|
|
||||||
existing = issue_lock_store.load_issue_lock(
|
|
||||||
remote=str(data.get("remote") or ""),
|
|
||||||
org=str(data.get("org") or ""),
|
|
||||||
repo=str(data.get("repo") or ""),
|
|
||||||
issue_number=int(data.get("issue_number") or 0),
|
|
||||||
)
|
|
||||||
overwrite_block = issue_lock_store.assess_foreign_lock_overwrite(existing, data)
|
|
||||||
if overwrite_block:
|
|
||||||
raise RuntimeError(overwrite_block)
|
|
||||||
try:
|
try:
|
||||||
return issue_lock_store.bind_session_lock(data)
|
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f:
|
||||||
except Exception as e:
|
data = json.load(f)
|
||||||
raise RuntimeError(f"Could not write issue lock file: {e}") from e
|
return data if isinstance(data, dict) else None
|
||||||
|
except Exception:
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def _work_lease_claimant(host: str | None) -> dict:
|
def _work_lease_claimant(host: str | None) -> dict:
|
||||||
@@ -723,132 +679,6 @@ def _branch_entry_name(branch: dict | str) -> str:
|
|||||||
return str(branch.get("name") or branch.get("ref") or "")
|
return str(branch.get("name") or branch.get("ref") or "")
|
||||||
|
|
||||||
|
|
||||||
def _live_fetch_issue_duplicate_context(
|
|
||||||
h: str,
|
|
||||||
o: str,
|
|
||||||
r: str,
|
|
||||||
auth: str,
|
|
||||||
issue_number: int,
|
|
||||||
) -> tuple[list[dict], list[str], dict]:
|
|
||||||
"""Live open PRs, remote branch names, and claim state for one issue."""
|
|
||||||
base = repo_api_url(h, o, r)
|
|
||||||
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
|
|
||||||
branches = api_get_all(f"{base}/branches", auth)
|
|
||||||
branch_names = [_branch_entry_name(b) for b in branches]
|
|
||||||
issue = api_request("GET", f"{base}/issues/{issue_number}", auth) or {}
|
|
||||||
comments = api_request(
|
|
||||||
"GET", f"{base}/issues/{issue_number}/comments", auth
|
|
||||||
) or []
|
|
||||||
claim_entry = issue_claim_heartbeat.classify_issue_claim(
|
|
||||||
issue=issue,
|
|
||||||
comments=comments,
|
|
||||||
open_prs=open_prs,
|
|
||||||
branch_names=branch_names,
|
|
||||||
)
|
|
||||||
return open_prs, branch_names, claim_entry
|
|
||||||
|
|
||||||
|
|
||||||
# Injectable duplicate-work context fetcher (#400). Production uses the live
|
|
||||||
# Gitea API path above; unit tests patch this symbol instead of hitting the
|
|
||||||
# network.
|
|
||||||
issue_duplicate_context_fetcher = _live_fetch_issue_duplicate_context
|
|
||||||
|
|
||||||
|
|
||||||
def _collect_issue_duplicate_context(
|
|
||||||
h: str,
|
|
||||||
o: str,
|
|
||||||
r: str,
|
|
||||||
auth: str,
|
|
||||||
issue_number: int,
|
|
||||||
) -> tuple[list[dict], list[str], dict]:
|
|
||||||
return issue_duplicate_context_fetcher(h, o, r, auth, issue_number)
|
|
||||||
|
|
||||||
|
|
||||||
def _assess_issue_duplicate_gate(
|
|
||||||
issue_number: int,
|
|
||||||
*,
|
|
||||||
h: str,
|
|
||||||
o: str,
|
|
||||||
r: str,
|
|
||||||
auth: str,
|
|
||||||
locked_branch: str | None = None,
|
|
||||||
phase: str,
|
|
||||||
) -> dict:
|
|
||||||
open_prs, branch_names, claim_entry = _collect_issue_duplicate_context(
|
|
||||||
h, o, r, auth, issue_number
|
|
||||||
)
|
|
||||||
return issue_work_duplicate_gate.assess_work_issue_duplicate_gate(
|
|
||||||
issue_number,
|
|
||||||
open_prs=open_prs,
|
|
||||||
branch_names=branch_names,
|
|
||||||
claim_entry=claim_entry,
|
|
||||||
locked_branch=locked_branch,
|
|
||||||
phase=phase,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _duplicate_gate_block_response(gate: dict, **extra) -> dict:
|
|
||||||
out = {
|
|
||||||
"success": False,
|
|
||||||
"performed": False,
|
|
||||||
"reasons": list(gate.get("reasons") or []),
|
|
||||||
"duplicate_gate": gate,
|
|
||||||
"safe_next_action": gate.get("safe_next_action"),
|
|
||||||
}
|
|
||||||
out.update(extra)
|
|
||||||
return out
|
|
||||||
|
|
||||||
|
|
||||||
def _enforce_locked_issue_duplicate_recheck(
|
|
||||||
remote: str,
|
|
||||||
phase: str,
|
|
||||||
*,
|
|
||||||
host: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
) -> dict | None:
|
|
||||||
"""Re-check duplicate-work gates for the locked issue (#400)."""
|
|
||||||
lock_data = _load_existing_issue_lock()
|
|
||||||
if not lock_data:
|
|
||||||
return None
|
|
||||||
issue_number = int(lock_data.get("issue_number") or 0)
|
|
||||||
locked_branch = lock_data.get("branch_name")
|
|
||||||
if not issue_number:
|
|
||||||
return None
|
|
||||||
h, o, r = _resolve(
|
|
||||||
remote or lock_data.get("remote") or "dadeschools",
|
|
||||||
host or lock_data.get("host"),
|
|
||||||
org or lock_data.get("org"),
|
|
||||||
repo or lock_data.get("repo"),
|
|
||||||
)
|
|
||||||
auth = _auth(h)
|
|
||||||
gate = _assess_issue_duplicate_gate(
|
|
||||||
issue_number,
|
|
||||||
h=h,
|
|
||||||
o=o,
|
|
||||||
r=r,
|
|
||||||
auth=auth,
|
|
||||||
locked_branch=locked_branch,
|
|
||||||
phase=phase,
|
|
||||||
)
|
|
||||||
if gate.get("block"):
|
|
||||||
return gate
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def _branch_entry_commit_sha(branch: dict | str) -> str | None:
|
|
||||||
"""Best-effort head SHA for a Gitea branch entry (None when absent)."""
|
|
||||||
if not isinstance(branch, dict):
|
|
||||||
return None
|
|
||||||
commit = branch.get("commit")
|
|
||||||
if isinstance(commit, dict):
|
|
||||||
sha = commit.get("id") or commit.get("sha")
|
|
||||||
if sha:
|
|
||||||
return str(sha)
|
|
||||||
sha = branch.get("commit_sha")
|
|
||||||
return str(sha) if sha else None
|
|
||||||
|
|
||||||
|
|
||||||
def _reveal_endpoints() -> bool:
|
def _reveal_endpoints() -> bool:
|
||||||
"""Admin/debug opt-in (#120): include endpoint URLs and token source
|
"""Admin/debug opt-in (#120): include endpoint URLs and token source
|
||||||
names in tool output. Off by default so normal LLM-facing responses
|
names in tool output. Off by default so normal LLM-facing responses
|
||||||
@@ -1296,9 +1126,8 @@ def gitea_lock_issue(
|
|||||||
resolved_worktree = issue_lock_worktree.resolve_author_worktree_path(
|
resolved_worktree = issue_lock_worktree.resolve_author_worktree_path(
|
||||||
worktree_path, PROJECT_ROOT
|
worktree_path, PROJECT_ROOT
|
||||||
)
|
)
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
active_lease_block = _active_work_lease_block(
|
||||||
active_lease_block = issue_lock_store.assess_same_issue_lease_conflict(
|
_load_existing_issue_lock(),
|
||||||
_load_existing_issue_lock(remote=remote, org=o, repo=r, issue_number=issue_number),
|
|
||||||
issue_number=issue_number,
|
issue_number=issue_number,
|
||||||
branch_name=branch_name,
|
branch_name=branch_name,
|
||||||
worktree_path=resolved_worktree,
|
worktree_path=resolved_worktree,
|
||||||
@@ -1322,43 +1151,47 @@ def gitea_lock_issue(
|
|||||||
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
|
issue_lock_worktree.format_issue_lock_worktree_error(lock_assessment)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# 2. Check if the issue already has an open PR (reuse protection)
|
||||||
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
auth = _auth(h)
|
auth = _auth(h)
|
||||||
duplicate_gate = _assess_issue_duplicate_gate(
|
url = f"{repo_api_url(h, o, r)}/pulls?state=open"
|
||||||
issue_number,
|
|
||||||
h=h,
|
try:
|
||||||
o=o,
|
prs = api_get_all(url, auth)
|
||||||
r=r,
|
except Exception as e:
|
||||||
auth=auth,
|
raise RuntimeError(f"Could not list open PRs to verify issue lock: {e}")
|
||||||
locked_branch=branch_name,
|
|
||||||
phase=issue_work_duplicate_gate.PHASE_LOCK,
|
for pr in prs:
|
||||||
|
pr_head = pr.get("head", {}).get("ref", "")
|
||||||
|
pr_title = pr.get("title", "")
|
||||||
|
pr_body = pr.get("body", "")
|
||||||
|
|
||||||
|
if expected_pattern in pr_head:
|
||||||
|
raise ValueError(
|
||||||
|
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}, branch '{pr_head}') (fail closed)"
|
||||||
|
)
|
||||||
|
|
||||||
|
patterns = [
|
||||||
|
f"closes #{issue_number}",
|
||||||
|
f"fixes #{issue_number}",
|
||||||
|
]
|
||||||
|
text_to_check = f"{pr_title} {pr_body}".lower()
|
||||||
|
if any(p in text_to_check for p in patterns):
|
||||||
|
raise ValueError(
|
||||||
|
f"Issue #{issue_number} is already tied to an open PR (PR #{pr.get('number')}) via Closes/Fixes reference (fail closed)"
|
||||||
)
|
)
|
||||||
if duplicate_gate.get("block"):
|
|
||||||
raise ValueError("; ".join(duplicate_gate.get("reasons") or [
|
|
||||||
f"duplicate work gate blocked issue #{issue_number} (fail closed)"
|
|
||||||
]))
|
|
||||||
|
|
||||||
branch_url = f"{repo_api_url(h, o, r)}/branches"
|
branch_url = f"{repo_api_url(h, o, r)}/branches"
|
||||||
try:
|
try:
|
||||||
branches = api_get_all(branch_url, auth)
|
branches = api_get_all(branch_url, auth)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise RuntimeError(f"Could not list branches to verify issue lock: {e}")
|
raise RuntimeError(f"Could not list branches to verify issue lock: {e}")
|
||||||
existing_branch_entries = [
|
for branch in branches:
|
||||||
{
|
name = _branch_entry_name(branch)
|
||||||
"name": _branch_entry_name(branch),
|
if expected_pattern in name:
|
||||||
"commit_sha": _branch_entry_commit_sha(branch),
|
|
||||||
}
|
|
||||||
for branch in branches
|
|
||||||
]
|
|
||||||
adoption = issue_lock_adoption.assess_own_branch_adoption(
|
|
||||||
issue_number=issue_number,
|
|
||||||
requested_branch=branch_name,
|
|
||||||
existing_branches=existing_branch_entries,
|
|
||||||
)
|
|
||||||
if adoption["block"]:
|
|
||||||
competing = ", ".join(adoption["competing_branches"])
|
|
||||||
raise ValueError(
|
raise ValueError(
|
||||||
f"Issue #{issue_number} already has matching branch '{competing}' "
|
f"Issue #{issue_number} already has matching branch '{name}' "
|
||||||
"that is not the requested branch (fail closed)"
|
"(fail closed)"
|
||||||
)
|
)
|
||||||
|
|
||||||
work_lease = _build_author_issue_work_lease(
|
work_lease = _build_author_issue_work_lease(
|
||||||
@@ -1375,13 +1208,13 @@ def gitea_lock_issue(
|
|||||||
"repo": r,
|
"repo": r,
|
||||||
"worktree_path": resolved_worktree,
|
"worktree_path": resolved_worktree,
|
||||||
"work_lease": work_lease,
|
"work_lease": work_lease,
|
||||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
|
||||||
tool="gitea_lock_issue",
|
|
||||||
claimant=work_lease.get("claimant"),
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
|
|
||||||
lock_file_path = _save_issue_lock(data)
|
try:
|
||||||
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
|
json.dump(data, f)
|
||||||
|
except Exception as e:
|
||||||
|
raise RuntimeError(f"Could not write issue lock file: {e}")
|
||||||
|
|
||||||
agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(
|
agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(
|
||||||
git_state.get("porcelain_status") or ""
|
git_state.get("porcelain_status") or ""
|
||||||
@@ -1396,22 +1229,7 @@ def gitea_lock_issue(
|
|||||||
"branch_name": branch_name,
|
"branch_name": branch_name,
|
||||||
"worktree_path": resolved_worktree,
|
"worktree_path": resolved_worktree,
|
||||||
"work_lease": work_lease,
|
"work_lease": work_lease,
|
||||||
"lock_file_path": lock_file_path,
|
|
||||||
}
|
}
|
||||||
if adoption["adopt"]:
|
|
||||||
result["adoption"] = issue_lock_adoption.build_adoption_proof(
|
|
||||||
issue_number=issue_number,
|
|
||||||
branch_name=branch_name,
|
|
||||||
assessment=adoption,
|
|
||||||
open_pr_checked=True,
|
|
||||||
competing_lock_checked=True,
|
|
||||||
lock_file_path=lock_file_path,
|
|
||||||
lock_file_status="written",
|
|
||||||
)
|
|
||||||
result["message"] = (
|
|
||||||
f"Adopted existing branch '{branch_name}' and locked issue "
|
|
||||||
f"#{issue_number} for recovery (fail-closed check complete)."
|
|
||||||
)
|
|
||||||
if agent_artifacts:
|
if agent_artifacts:
|
||||||
result["warnings"] = [
|
result["warnings"] = [
|
||||||
"Agent temp artifacts at repo root (delete before implementation): "
|
"Agent temp artifacts at repo root (delete before implementation): "
|
||||||
@@ -1420,39 +1238,6 @@ def gitea_lock_issue(
|
|||||||
return result
|
return result
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
|
||||||
def gitea_assess_work_issue_duplicate(
|
|
||||||
issue_number: int,
|
|
||||||
branch_name: str | None = None,
|
|
||||||
phase: str = issue_work_duplicate_gate.PHASE_LOCK,
|
|
||||||
remote: str = "dadeschools",
|
|
||||||
host: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Read-only duplicate-work gate for author sessions before mutations (#400)."""
|
|
||||||
read_block = _profile_operation_gate("gitea.read")
|
|
||||||
if read_block:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"performed": False,
|
|
||||||
"reasons": read_block,
|
|
||||||
"permission_report": _permission_block_report("gitea.read"),
|
|
||||||
}
|
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
|
||||||
auth = _auth(h)
|
|
||||||
gate = _assess_issue_duplicate_gate(
|
|
||||||
issue_number,
|
|
||||||
h=h,
|
|
||||||
o=o,
|
|
||||||
r=r,
|
|
||||||
auth=auth,
|
|
||||||
locked_branch=branch_name,
|
|
||||||
phase=phase,
|
|
||||||
)
|
|
||||||
return {"success": not gate.get("block"), **gate}
|
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_create_pr(
|
def gitea_create_pr(
|
||||||
title: str,
|
title: str,
|
||||||
@@ -1503,16 +1288,15 @@ def gitea_create_pr(
|
|||||||
verify_preflight_purity(remote, worktree_path=worktree_path)
|
verify_preflight_purity(remote, worktree_path=worktree_path)
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
h, o, r = _resolve(remote, host, org, repo)
|
||||||
|
|
||||||
# ── Issue Lock Validation (Issue #194 / #196 / #443) ──
|
# ── Issue Lock Validation (Issue #194 / #196) ──
|
||||||
lock_data = _resolve_issue_lock_for_pr(remote=remote, org=o, repo=r, head=head)
|
if not os.path.exists(ISSUE_LOCK_FILE):
|
||||||
|
raise RuntimeError("Issue lock is missing (fail closed). Call gitea_lock_issue first.")
|
||||||
|
|
||||||
lock_provenance_check = issue_lock_provenance.assess_lock_file_for_create_pr(
|
try:
|
||||||
lock_data
|
with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f:
|
||||||
)
|
lock_data = json.load(f)
|
||||||
if lock_provenance_check["block"]:
|
except Exception as e:
|
||||||
raise RuntimeError(
|
raise RuntimeError(f"Could not read issue lock file: {e} (fail closed)")
|
||||||
issue_lock_provenance.format_lock_provenance_error(lock_provenance_check)
|
|
||||||
)
|
|
||||||
|
|
||||||
locked_issue = lock_data.get("issue_number")
|
locked_issue = lock_data.get("issue_number")
|
||||||
locked_branch = lock_data.get("branch_name")
|
locked_branch = lock_data.get("branch_name")
|
||||||
@@ -1545,21 +1329,6 @@ def gitea_create_pr(
|
|||||||
f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)"
|
f"PR title or body must contain 'Closes #{locked_issue}' or 'Fixes #{locked_issue}' exactly to ensure durable tracking (fail closed)"
|
||||||
)
|
)
|
||||||
|
|
||||||
duplicate_block = _enforce_locked_issue_duplicate_recheck(
|
|
||||||
remote,
|
|
||||||
issue_work_duplicate_gate.PHASE_CREATE_PR,
|
|
||||||
host=host,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
)
|
|
||||||
if duplicate_block:
|
|
||||||
return _duplicate_gate_block_response(
|
|
||||||
duplicate_block,
|
|
||||||
number=None,
|
|
||||||
issue_number=locked_issue,
|
|
||||||
branch_name=locked_branch,
|
|
||||||
)
|
|
||||||
|
|
||||||
auth = _auth(h)
|
auth = _auth(h)
|
||||||
url = f"{repo_api_url(h, o, r)}/pulls"
|
url = f"{repo_api_url(h, o, r)}/pulls"
|
||||||
payload = {"title": title, "body": body, "head": head, "base": base}
|
payload = {"title": title, "body": body, "head": head, "base": base}
|
||||||
@@ -2099,7 +1868,6 @@ def init_review_decision_lock(remote: str | None, task: str | None):
|
|||||||
(os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
(os.environ.get(SESSION_PROFILE_LOCK_ENV) or "").strip()
|
||||||
or profile_name
|
or profile_name
|
||||||
)
|
)
|
||||||
reviewer_pr_lease.clear_session_lease()
|
|
||||||
_save_review_decision_lock({
|
_save_review_decision_lock({
|
||||||
"task": task,
|
"task": task,
|
||||||
"remote": remote,
|
"remote": remote,
|
||||||
@@ -2542,20 +2310,6 @@ def _evaluate_pr_review_submission(
|
|||||||
result["permission_report"] = elig["permission_report"]
|
result["permission_report"] = elig["permission_report"]
|
||||||
return result
|
return result
|
||||||
|
|
||||||
if live:
|
|
||||||
reasons.extend(_reviewer_pr_lease_gate(
|
|
||||||
pr_number=pr_number,
|
|
||||||
remote=remote,
|
|
||||||
host=host,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
mutation=action,
|
|
||||||
live_head_sha=result.get("head_sha"),
|
|
||||||
pinned_head_sha=expected_head_sha,
|
|
||||||
))
|
|
||||||
if reasons:
|
|
||||||
return result
|
|
||||||
|
|
||||||
auth_user = result["authenticated_user"]
|
auth_user = result["authenticated_user"]
|
||||||
pr_author = result["pr_author"]
|
pr_author = result["pr_author"]
|
||||||
if action == "approve" and auth_user and pr_author and auth_user == pr_author:
|
if action == "approve" and auth_user and pr_author and auth_user == pr_author:
|
||||||
@@ -3038,7 +2792,13 @@ def _prepare_commit_payload_files(files: list[dict]) -> tuple[list[dict], list[d
|
|||||||
processed_files = []
|
processed_files = []
|
||||||
source_proofs = []
|
source_proofs = []
|
||||||
|
|
||||||
lock_data = issue_lock_store.read_session_issue_lock() or {}
|
lock_data = {}
|
||||||
|
if os.path.exists(ISSUE_LOCK_FILE):
|
||||||
|
try:
|
||||||
|
with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f:
|
||||||
|
lock_data = json.load(f)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
locked_worktree = lock_data.get("worktree_path")
|
locked_worktree = lock_data.get("worktree_path")
|
||||||
if locked_worktree:
|
if locked_worktree:
|
||||||
@@ -3173,20 +2933,6 @@ def gitea_commit_files(
|
|||||||
if blocked:
|
if blocked:
|
||||||
return blocked
|
return blocked
|
||||||
|
|
||||||
duplicate_block = _enforce_locked_issue_duplicate_recheck(
|
|
||||||
remote,
|
|
||||||
issue_work_duplicate_gate.PHASE_COMMIT,
|
|
||||||
host=host,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
)
|
|
||||||
if duplicate_block:
|
|
||||||
return _duplicate_gate_block_response(
|
|
||||||
duplicate_block,
|
|
||||||
commit="",
|
|
||||||
branch="",
|
|
||||||
)
|
|
||||||
|
|
||||||
verify_preflight_purity(remote)
|
verify_preflight_purity(remote)
|
||||||
processed_files, source_proofs = _prepare_commit_payload_files(files)
|
processed_files, source_proofs = _prepare_commit_payload_files(files)
|
||||||
|
|
||||||
@@ -3349,19 +3095,6 @@ def gitea_merge_pr(
|
|||||||
result["permission_report"] = elig["permission_report"]
|
result["permission_report"] = elig["permission_report"]
|
||||||
return result
|
return result
|
||||||
|
|
||||||
reasons.extend(_reviewer_pr_lease_gate(
|
|
||||||
pr_number=pr_number,
|
|
||||||
remote=remote,
|
|
||||||
host=host,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
mutation="merge",
|
|
||||||
live_head_sha=result.get("head_sha"),
|
|
||||||
pinned_head_sha=expected_head_sha,
|
|
||||||
))
|
|
||||||
if reasons:
|
|
||||||
return result
|
|
||||||
|
|
||||||
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
|
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
|
||||||
actual_sha = result["head_sha"]
|
actual_sha = result["head_sha"]
|
||||||
if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
|
if expected_head_sha and actual_sha and expected_head_sha != actual_sha:
|
||||||
@@ -4661,261 +4394,6 @@ def _namespace_mutation_block(mutation_task: str, **extra_fields) -> dict | None
|
|||||||
return blocked
|
return blocked
|
||||||
|
|
||||||
|
|
||||||
def _fetch_pr_comments(
|
|
||||||
pr_number: int,
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
host: str | None,
|
|
||||||
org: str | None,
|
|
||||||
repo: str | None,
|
|
||||||
) -> list[dict]:
|
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
|
||||||
auth = _auth(h)
|
|
||||||
api = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
|
||||||
return api_request("GET", api, auth) or []
|
|
||||||
|
|
||||||
|
|
||||||
def _reviewer_pr_lease_gate(
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
remote: str,
|
|
||||||
host: str | None,
|
|
||||||
org: str | None,
|
|
||||||
repo: str | None,
|
|
||||||
mutation: str,
|
|
||||||
live_head_sha: str | None,
|
|
||||||
pinned_head_sha: str | None,
|
|
||||||
) -> list[str]:
|
|
||||||
"""Return block reasons when the session lacks an owned PR reviewer lease."""
|
|
||||||
session = reviewer_pr_lease.get_session_lease()
|
|
||||||
session_id = (session or {}).get("session_id")
|
|
||||||
identity = _authenticated_username(remote) or ""
|
|
||||||
try:
|
|
||||||
comments = _fetch_pr_comments(
|
|
||||||
pr_number, remote=remote, host=host, org=org, repo=repo)
|
|
||||||
except Exception as exc:
|
|
||||||
return [f"cannot fetch PR comments for lease gate: {_redact(str(exc))}"]
|
|
||||||
assessment = reviewer_pr_lease.assess_mutation_lease_gate(
|
|
||||||
pr_number=pr_number,
|
|
||||||
comments=comments,
|
|
||||||
reviewer_identity=identity,
|
|
||||||
session_id=session_id,
|
|
||||||
mutation=mutation,
|
|
||||||
live_head_sha=live_head_sha,
|
|
||||||
pinned_head_sha=pinned_head_sha,
|
|
||||||
)
|
|
||||||
return list(assessment.get("reasons") or []) if assessment.get("block") else []
|
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
|
||||||
def gitea_acquire_reviewer_pr_lease(
|
|
||||||
pr_number: int,
|
|
||||||
worktree: str,
|
|
||||||
candidate_head: str | None = None,
|
|
||||||
target_branch: str = "master",
|
|
||||||
target_branch_sha: str | None = None,
|
|
||||||
issue_number: int | None = None,
|
|
||||||
session_id: str | None = None,
|
|
||||||
remote: str = "dadeschools",
|
|
||||||
host: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Acquire a per-PR reviewer lease before review/merge mutations (#407)."""
|
|
||||||
read_block = _profile_operation_gate("gitea.read")
|
|
||||||
if read_block:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"acquired": False,
|
|
||||||
"reasons": read_block,
|
|
||||||
"permission_report": _permission_block_report("gitea.read"),
|
|
||||||
}
|
|
||||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
|
||||||
if comment_block:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"acquired": False,
|
|
||||||
"reasons": comment_block,
|
|
||||||
"permission_report": _permission_block_report("gitea.pr.comment"),
|
|
||||||
}
|
|
||||||
|
|
||||||
verify_preflight_purity(remote)
|
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
|
||||||
auth = _auth(h)
|
|
||||||
profile = get_profile()
|
|
||||||
identity = _authenticated_username(remote) or profile.get("username") or ""
|
|
||||||
sid = (session_id or "").strip() or reviewer_pr_lease.new_session_id()
|
|
||||||
repo_label = f"{o}/{r}"
|
|
||||||
|
|
||||||
comments = _fetch_pr_comments(
|
|
||||||
pr_number, remote=remote, host=host, org=org, repo=repo)
|
|
||||||
assessment = reviewer_pr_lease.assess_acquire_lease(
|
|
||||||
comments,
|
|
||||||
pr_number=pr_number,
|
|
||||||
reviewer_identity=identity,
|
|
||||||
profile=profile.get("profile_name") or "unknown",
|
|
||||||
session_id=sid,
|
|
||||||
repo=repo_label,
|
|
||||||
issue_number=issue_number,
|
|
||||||
worktree=worktree,
|
|
||||||
candidate_head=candidate_head,
|
|
||||||
target_branch=target_branch,
|
|
||||||
target_branch_sha=target_branch_sha,
|
|
||||||
)
|
|
||||||
if not assessment.get("acquire_allowed"):
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"acquired": False,
|
|
||||||
"reasons": assessment.get("reasons") or [],
|
|
||||||
"existing_lease": assessment.get("existing_lease"),
|
|
||||||
}
|
|
||||||
|
|
||||||
body = assessment["lease_body"]
|
|
||||||
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
|
||||||
with _audited(
|
|
||||||
"comment_pr",
|
|
||||||
host=h,
|
|
||||||
remote=remote,
|
|
||||||
org=o,
|
|
||||||
repo=r,
|
|
||||||
pr_number=pr_number,
|
|
||||||
request_metadata={"source": "acquire_reviewer_pr_lease"},
|
|
||||||
):
|
|
||||||
posted = api_request("POST", comment_url, auth, {"body": body})
|
|
||||||
|
|
||||||
session_lease = reviewer_pr_lease.record_session_lease({
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"session_id": sid,
|
|
||||||
"reviewer_identity": identity,
|
|
||||||
"profile": profile.get("profile_name"),
|
|
||||||
"worktree": worktree,
|
|
||||||
"phase": "claimed",
|
|
||||||
"candidate_head": candidate_head,
|
|
||||||
"target_branch": target_branch,
|
|
||||||
"target_branch_sha": target_branch_sha,
|
|
||||||
"repo": repo_label,
|
|
||||||
"comment_id": posted.get("id"),
|
|
||||||
})
|
|
||||||
return {
|
|
||||||
"success": True,
|
|
||||||
"acquired": True,
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"session_id": sid,
|
|
||||||
"comment_id": posted.get("id"),
|
|
||||||
"session_lease": session_lease,
|
|
||||||
"reasons": [],
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
|
||||||
def gitea_heartbeat_reviewer_pr_lease(
|
|
||||||
pr_number: int,
|
|
||||||
phase: str,
|
|
||||||
worktree: str | None = None,
|
|
||||||
candidate_head: str | None = None,
|
|
||||||
target_branch_sha: str | None = None,
|
|
||||||
remote: str = "dadeschools",
|
|
||||||
host: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Post a reviewer lease heartbeat / phase update on the PR thread (#407)."""
|
|
||||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
|
||||||
if comment_block:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"posted": False,
|
|
||||||
"reasons": comment_block,
|
|
||||||
"permission_report": _permission_block_report("gitea.pr.comment"),
|
|
||||||
}
|
|
||||||
session = reviewer_pr_lease.get_session_lease()
|
|
||||||
if not session or session.get("pr_number") != pr_number:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"posted": False,
|
|
||||||
"reasons": [
|
|
||||||
f"no in-session lease for PR #{pr_number}; acquire first "
|
|
||||||
"(fail closed)"
|
|
||||||
],
|
|
||||||
}
|
|
||||||
|
|
||||||
verify_preflight_purity(remote)
|
|
||||||
h, o, r = _resolve(remote, host, org, repo)
|
|
||||||
auth = _auth(h)
|
|
||||||
body = reviewer_pr_lease.format_lease_body(
|
|
||||||
repo=f"{o}/{r}",
|
|
||||||
pr_number=pr_number,
|
|
||||||
issue_number=session.get("issue_number"),
|
|
||||||
reviewer_identity=session.get("reviewer_identity") or "",
|
|
||||||
profile=session.get("profile") or "unknown",
|
|
||||||
session_id=session.get("session_id") or reviewer_pr_lease.new_session_id(),
|
|
||||||
worktree=worktree or session.get("worktree") or "",
|
|
||||||
phase=phase,
|
|
||||||
candidate_head=candidate_head or session.get("candidate_head"),
|
|
||||||
target_branch=session.get("target_branch") or "master",
|
|
||||||
target_branch_sha=target_branch_sha or session.get("target_branch_sha"),
|
|
||||||
)
|
|
||||||
comment_url = f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments"
|
|
||||||
with _audited(
|
|
||||||
"comment_pr",
|
|
||||||
host=h,
|
|
||||||
remote=remote,
|
|
||||||
org=o,
|
|
||||||
repo=r,
|
|
||||||
pr_number=pr_number,
|
|
||||||
request_metadata={"source": "heartbeat_reviewer_pr_lease", "phase": phase},
|
|
||||||
):
|
|
||||||
posted = api_request("POST", comment_url, auth, {"body": body})
|
|
||||||
|
|
||||||
updated = reviewer_pr_lease.record_session_lease({
|
|
||||||
**session,
|
|
||||||
"phase": phase,
|
|
||||||
"worktree": worktree or session.get("worktree"),
|
|
||||||
"candidate_head": candidate_head or session.get("candidate_head"),
|
|
||||||
"target_branch_sha": target_branch_sha or session.get("target_branch_sha"),
|
|
||||||
"last_comment_id": posted.get("id"),
|
|
||||||
})
|
|
||||||
return {
|
|
||||||
"success": True,
|
|
||||||
"posted": True,
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"phase": phase,
|
|
||||||
"comment_id": posted.get("id"),
|
|
||||||
"session_lease": updated,
|
|
||||||
"reasons": [],
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
|
||||||
def gitea_assess_reviewer_pr_lease(
|
|
||||||
pr_number: int,
|
|
||||||
remote: str = "dadeschools",
|
|
||||||
host: str | None = None,
|
|
||||||
org: str | None = None,
|
|
||||||
repo: str | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Read-only: assess active reviewer lease state for a PR (#407)."""
|
|
||||||
read_block = _profile_operation_gate("gitea.read")
|
|
||||||
if read_block:
|
|
||||||
return {
|
|
||||||
"success": False,
|
|
||||||
"reasons": read_block,
|
|
||||||
"permission_report": _permission_block_report("gitea.read"),
|
|
||||||
}
|
|
||||||
comments = _fetch_pr_comments(
|
|
||||||
pr_number, remote=remote, host=host, org=org, repo=repo)
|
|
||||||
active = reviewer_pr_lease.find_active_reviewer_lease(
|
|
||||||
comments, pr_number=pr_number)
|
|
||||||
return {
|
|
||||||
"success": True,
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"active_lease": active,
|
|
||||||
"session_lease": reviewer_pr_lease.get_session_lease(),
|
|
||||||
"reasons": [],
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
def gitea_list_issue_comments(
|
def gitea_list_issue_comments(
|
||||||
issue_number: int,
|
issue_number: int,
|
||||||
|
|||||||
@@ -1,156 +0,0 @@
|
|||||||
"""Own-branch lock adoption / recovery for ``gitea_lock_issue`` (#442 / #443).
|
|
||||||
|
|
||||||
When an issue's own already-pushed branch exists, lock reacquisition must be
|
|
||||||
allowed (adoption) instead of being treated as #400 duplicate competing work.
|
|
||||||
This module isolates the pure decision so it can be unit-tested apart from the
|
|
||||||
MCP server's live Gitea calls.
|
|
||||||
|
|
||||||
Adoption is granted only for the issue's *exact* requested branch. Any other
|
|
||||||
branch that merely contains the same ``issue-<n>`` marker is competing work and
|
|
||||||
stays fail-closed. Open-PR, competing-live-lock, capability, and worktree
|
|
||||||
safety checks are enforced by the caller before this decision is consulted;
|
|
||||||
this module additionally records whether they passed for proof purposes.
|
|
||||||
"""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import re
|
|
||||||
|
|
||||||
ADOPT = "adopt_existing_branch"
|
|
||||||
BLOCK_COMPETING = "block_competing_branch"
|
|
||||||
NO_MATCH = "no_matching_branch"
|
|
||||||
|
|
||||||
|
|
||||||
def _branch_name(entry) -> str:
|
|
||||||
if isinstance(entry, dict):
|
|
||||||
return str(entry.get("name") or "")
|
|
||||||
return str(entry or "")
|
|
||||||
|
|
||||||
|
|
||||||
def _branch_sha(entry) -> str | None:
|
|
||||||
if isinstance(entry, dict):
|
|
||||||
sha = entry.get("commit_sha")
|
|
||||||
if sha:
|
|
||||||
return str(sha)
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def _branch_carries_issue_marker(branch_name: str, issue_number: int) -> bool:
|
|
||||||
"""Return True when *branch_name* references issue *issue_number* exactly.
|
|
||||||
|
|
||||||
Uses a numeric word-boundary so ``issue-42`` does not match inside
|
|
||||||
``issue-420`` (AC6 / #440).
|
|
||||||
"""
|
|
||||||
name = (branch_name or "").strip()
|
|
||||||
if not name:
|
|
||||||
return False
|
|
||||||
pattern = rf"(?:^|/)issue-{int(issue_number)}(?![0-9])"
|
|
||||||
return re.search(pattern, name) is not None
|
|
||||||
|
|
||||||
|
|
||||||
def assess_own_branch_adoption(
|
|
||||||
*,
|
|
||||||
issue_number: int,
|
|
||||||
requested_branch: str,
|
|
||||||
existing_branches,
|
|
||||||
) -> dict:
|
|
||||||
"""Decide whether an existing matching branch is adoptable.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
issue_number: The tracking issue number being locked.
|
|
||||||
requested_branch: The exact branch the caller wants to lock.
|
|
||||||
existing_branches: Iterable of remote branch entries — either names or
|
|
||||||
dicts with ``name`` and optional ``commit_sha``.
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
dict with:
|
|
||||||
* ``outcome`` — one of ADOPT / BLOCK_COMPETING / NO_MATCH
|
|
||||||
* ``adopt`` (bool), ``block`` (bool)
|
|
||||||
* ``reason`` (str)
|
|
||||||
* ``matched_branch`` (str | None), ``matched_head_sha`` (str | None)
|
|
||||||
* ``competing_branches`` (list[str])
|
|
||||||
|
|
||||||
ADOPT: the issue's exact branch exists and no other same-issue branch does.
|
|
||||||
BLOCK_COMPETING: at least one same-issue branch is not the requested branch.
|
|
||||||
NO_MATCH: no branch carries the issue marker — normal lock path applies.
|
|
||||||
"""
|
|
||||||
requested = (requested_branch or "").strip()
|
|
||||||
|
|
||||||
matches: list[tuple[str, str | None]] = []
|
|
||||||
for entry in existing_branches or []:
|
|
||||||
name = _branch_name(entry).strip()
|
|
||||||
if _branch_carries_issue_marker(name, issue_number):
|
|
||||||
matches.append((name, _branch_sha(entry)))
|
|
||||||
|
|
||||||
competing = sorted({name for name, _ in matches if name != requested})
|
|
||||||
exact = [(name, sha) for name, sha in matches if name == requested]
|
|
||||||
|
|
||||||
# Fail closed whenever any non-requested same-issue branch exists, even if
|
|
||||||
# the requested branch is also present: ownership is then ambiguous.
|
|
||||||
if competing:
|
|
||||||
return {
|
|
||||||
"outcome": BLOCK_COMPETING,
|
|
||||||
"adopt": False,
|
|
||||||
"block": True,
|
|
||||||
"reason": (
|
|
||||||
f"issue #{issue_number} already has matching branch(es) "
|
|
||||||
f"{competing} that are not the requested branch "
|
|
||||||
f"'{requested}' (fail closed)"
|
|
||||||
),
|
|
||||||
"matched_branch": None,
|
|
||||||
"matched_head_sha": None,
|
|
||||||
"competing_branches": competing,
|
|
||||||
}
|
|
||||||
|
|
||||||
if exact:
|
|
||||||
name, sha = exact[0]
|
|
||||||
return {
|
|
||||||
"outcome": ADOPT,
|
|
||||||
"adopt": True,
|
|
||||||
"block": False,
|
|
||||||
"reason": (
|
|
||||||
f"existing branch '{name}' is the exact requested branch for "
|
|
||||||
f"issue #{issue_number}; adopting it for lock recovery"
|
|
||||||
),
|
|
||||||
"matched_branch": name,
|
|
||||||
"matched_head_sha": sha,
|
|
||||||
"competing_branches": [],
|
|
||||||
}
|
|
||||||
|
|
||||||
return {
|
|
||||||
"outcome": NO_MATCH,
|
|
||||||
"adopt": False,
|
|
||||||
"block": False,
|
|
||||||
"reason": f"no existing branch matches issue #{issue_number}",
|
|
||||||
"matched_branch": None,
|
|
||||||
"matched_head_sha": None,
|
|
||||||
"competing_branches": [],
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def build_adoption_proof(
|
|
||||||
*,
|
|
||||||
issue_number: int,
|
|
||||||
branch_name: str,
|
|
||||||
assessment: dict,
|
|
||||||
open_pr_checked: bool,
|
|
||||||
competing_lock_checked: bool,
|
|
||||||
lock_file_path: str,
|
|
||||||
lock_file_status: str,
|
|
||||||
) -> dict:
|
|
||||||
"""Assemble the proof block returned by ``gitea_lock_issue`` on adoption.
|
|
||||||
|
|
||||||
Requirement #4: adoption results must carry issue number, branch name,
|
|
||||||
branch head commit, adoption reason, no-existing-PR proof, no-competing-
|
|
||||||
live-lock proof, and lock file path/status.
|
|
||||||
"""
|
|
||||||
return {
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"branch_name": branch_name,
|
|
||||||
"branch_head_commit": assessment.get("matched_head_sha"),
|
|
||||||
"adoption_reason": assessment.get("reason"),
|
|
||||||
"no_existing_pr_proof": bool(open_pr_checked),
|
|
||||||
"no_competing_live_lock_proof": bool(competing_lock_checked),
|
|
||||||
"lock_file_path": lock_file_path,
|
|
||||||
"lock_file_status": lock_file_status,
|
|
||||||
}
|
|
||||||
@@ -1,269 +0,0 @@
|
|||||||
"""Issue-lock provenance and external-state disclosure (#447).
|
|
||||||
|
|
||||||
Sanctioned locks are written only by ``gitea_lock_issue`` (or adoption recovery
|
|
||||||
#442). Manual seeding of ``/tmp/gitea_issue_lock.json`` is unsafe and must be
|
|
||||||
blocked at PR creation unless explicit operator override proof is recorded.
|
|
||||||
"""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import os
|
|
||||||
import re
|
|
||||||
from datetime import datetime, timezone
|
|
||||||
|
|
||||||
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
|
|
||||||
|
|
||||||
SOURCE_LOCK_ISSUE = "gitea_lock_issue"
|
|
||||||
SOURCE_LOCK_ADOPTION = "gitea_lock_issue_adoption"
|
|
||||||
SOURCE_OPERATOR_OVERRIDE = "operator_override"
|
|
||||||
|
|
||||||
SANCTIONED_LOCK_SOURCES = frozenset({
|
|
||||||
SOURCE_LOCK_ISSUE,
|
|
||||||
SOURCE_LOCK_ADOPTION,
|
|
||||||
SOURCE_OPERATOR_OVERRIDE,
|
|
||||||
})
|
|
||||||
|
|
||||||
_OPERATOR_OVERRIDE_ENV = "GITEA_ISSUE_LOCK_OPERATOR_OVERRIDE"
|
|
||||||
|
|
||||||
_ISSUE_LOCK_PATH_RE = re.compile(
|
|
||||||
r"(?:/tmp/)?gitea_issue_lock\.json",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_LOCK_SEED_RE = re.compile(
|
|
||||||
r"(?:seed(?:ed|ing)?|restor(?:e|ed|ing)|wrote|written|write|programmatically|"
|
|
||||||
r"hand[- ]forg|manual(?:ly)?).{0,80}gitea_issue_lock",
|
|
||||||
re.IGNORECASE | re.DOTALL,
|
|
||||||
)
|
|
||||||
_LOCK_REMOVE_RE = re.compile(
|
|
||||||
r"(?:\brm\b|remove|deleted?|unlink).{0,80}gitea_issue_lock",
|
|
||||||
re.IGNORECASE | re.DOTALL,
|
|
||||||
)
|
|
||||||
_LOCK_READ_RE = re.compile(
|
|
||||||
r"(?:read|loaded?|parsed?).{0,80}gitea_issue_lock",
|
|
||||||
re.IGNORECASE | re.DOTALL,
|
|
||||||
)
|
|
||||||
_EXTERNAL_NONE_RE = re.compile(
|
|
||||||
r"external[- ]state mutations\s*:\s*none\b",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_EXTERNAL_FIELD_RE = re.compile(
|
|
||||||
r"external[- ]state mutations\s*:\s*(.+)$",
|
|
||||||
re.IGNORECASE | re.MULTILINE,
|
|
||||||
)
|
|
||||||
_CLEANUP_ONLY_RE = re.compile(
|
|
||||||
r"cleanup mutations\s*:\s*(?:none|lock removed|removed issue lock)",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_PR_CREATED_RE = re.compile(
|
|
||||||
r"(?:\bgitea_create_pr\b|PR\s*#\s*\d+\s+created|created\s+PR\s*#|opened\s+PR\s*#|"
|
|
||||||
r"PR\s+creation\s+(?:succeeded|complete))",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_REVIEW_APPROVE_RE = re.compile(
|
|
||||||
r"(?:submitted\s+(?:['\"]approve['\"]|approve\s+review)|"
|
|
||||||
r"review decision\s*:\s*approve|approved\s+PR\s*#|gitea_review_pr.*approve)",
|
|
||||||
re.IGNORECASE,
|
|
||||||
)
|
|
||||||
_OVERRIDE_PROOF_RE = re.compile(
|
|
||||||
r"operator[- ]override\s+proof\s*:\s*(.+)$",
|
|
||||||
re.IGNORECASE | re.MULTILINE,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _utc_now_iso() -> str:
|
|
||||||
return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
|
|
||||||
|
|
||||||
|
|
||||||
def build_sanctioned_lock_provenance(
|
|
||||||
*,
|
|
||||||
tool: str,
|
|
||||||
source: str = SOURCE_LOCK_ISSUE,
|
|
||||||
claimant: dict | None = None,
|
|
||||||
adoption: dict | None = None,
|
|
||||||
) -> dict:
|
|
||||||
"""Return provenance metadata stored with a sanctioned lock write."""
|
|
||||||
entry = {
|
|
||||||
"source": source,
|
|
||||||
"written_at": _utc_now_iso(),
|
|
||||||
"written_by_tool": tool,
|
|
||||||
"lock_file_path": ISSUE_LOCK_FILE,
|
|
||||||
}
|
|
||||||
if claimant:
|
|
||||||
entry["claimant"] = claimant
|
|
||||||
if adoption:
|
|
||||||
entry["adoption"] = adoption
|
|
||||||
return entry
|
|
||||||
|
|
||||||
|
|
||||||
def operator_override_requested() -> bool:
|
|
||||||
return os.environ.get(_OPERATOR_OVERRIDE_ENV, "").strip().lower() in {
|
|
||||||
"1",
|
|
||||||
"true",
|
|
||||||
"yes",
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def build_operator_override_provenance(*, reason: str, claimant: dict | None = None) -> dict:
|
|
||||||
text = (reason or "").strip()
|
|
||||||
if not text:
|
|
||||||
raise ValueError(
|
|
||||||
"operator override requires a non-empty override reason (fail closed)"
|
|
||||||
)
|
|
||||||
entry = build_sanctioned_lock_provenance(
|
|
||||||
tool="operator_override",
|
|
||||||
source=SOURCE_OPERATOR_OVERRIDE,
|
|
||||||
claimant=claimant,
|
|
||||||
)
|
|
||||||
entry["override_reason"] = text
|
|
||||||
return entry
|
|
||||||
|
|
||||||
|
|
||||||
def assess_lock_file_for_create_pr(lock_data: dict | None) -> dict:
|
|
||||||
"""Fail closed when lock file lacks sanctioned provenance (#447)."""
|
|
||||||
data = lock_data if isinstance(lock_data, dict) else {}
|
|
||||||
reasons: list[str] = []
|
|
||||||
provenance = data.get("lock_provenance")
|
|
||||||
if not isinstance(provenance, dict):
|
|
||||||
reasons.append(
|
|
||||||
"issue lock file lacks sanctioned lock_provenance; manual seeding is "
|
|
||||||
"not a normal recovery path — call gitea_lock_issue or use #442 adoption"
|
|
||||||
)
|
|
||||||
return _provenance_result(False, reasons, provenance)
|
|
||||||
|
|
||||||
source = str(provenance.get("source") or "").strip()
|
|
||||||
if source not in SANCTIONED_LOCK_SOURCES:
|
|
||||||
reasons.append(
|
|
||||||
f"issue lock provenance source '{source or '(missing)'}' is not sanctioned"
|
|
||||||
)
|
|
||||||
|
|
||||||
if source == SOURCE_OPERATOR_OVERRIDE and not str(
|
|
||||||
provenance.get("override_reason") or ""
|
|
||||||
).strip():
|
|
||||||
reasons.append(
|
|
||||||
"operator_override lock provenance requires override_reason proof"
|
|
||||||
)
|
|
||||||
|
|
||||||
if not data.get("work_lease"):
|
|
||||||
reasons.append("issue lock file missing work_lease metadata")
|
|
||||||
|
|
||||||
if not str(provenance.get("written_by_tool") or "").strip():
|
|
||||||
reasons.append("issue lock provenance missing written_by_tool")
|
|
||||||
|
|
||||||
proven = not reasons
|
|
||||||
return _provenance_result(proven, reasons, provenance)
|
|
||||||
|
|
||||||
|
|
||||||
def _provenance_result(proven: bool, reasons: list[str], provenance: dict | None) -> dict:
|
|
||||||
return {
|
|
||||||
"proven": proven,
|
|
||||||
"block": not proven,
|
|
||||||
"reasons": reasons,
|
|
||||||
"lock_provenance": provenance,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def format_lock_provenance_error(assessment: dict) -> str:
|
|
||||||
reasons = "; ".join(assessment.get("reasons") or ["unknown lock provenance violation"])
|
|
||||||
return f"Issue lock provenance guard (#447): {reasons} (fail closed)"
|
|
||||||
|
|
||||||
|
|
||||||
def _lock_activity_detected(text: str) -> dict[str, bool]:
|
|
||||||
body = text or ""
|
|
||||||
return {
|
|
||||||
"seed_or_restore": bool(_LOCK_SEED_RE.search(body)),
|
|
||||||
"remove": bool(_LOCK_REMOVE_RE.search(body)),
|
|
||||||
"read": bool(_LOCK_READ_RE.search(body)),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def _external_state_discloses_lock(text: str) -> bool:
|
|
||||||
match = _EXTERNAL_FIELD_RE.search(text or "")
|
|
||||||
if not match:
|
|
||||||
return False
|
|
||||||
value = (match.group(1) or "").strip().lower()
|
|
||||||
if value in {"", "none", "n/a"}:
|
|
||||||
return False
|
|
||||||
return "lock" in value or "gitea_issue_lock" in value or "issue-lock" in value
|
|
||||||
|
|
||||||
|
|
||||||
def assess_issue_lock_external_state_report(report_text: str) -> dict:
|
|
||||||
"""Require explicit external-state disclosure for issue-lock mutations (#447)."""
|
|
||||||
text = report_text or ""
|
|
||||||
activity = _lock_activity_detected(text)
|
|
||||||
if not any(activity.values()):
|
|
||||||
return {"proven": True, "block": False, "reasons": [], "activity": activity}
|
|
||||||
|
|
||||||
reasons: list[str] = []
|
|
||||||
disclosed = _external_state_discloses_lock(text)
|
|
||||||
|
|
||||||
if activity["seed_or_restore"] and _EXTERNAL_NONE_RE.search(text):
|
|
||||||
reasons.append(
|
|
||||||
"report mentions seeding/restoring gitea_issue_lock.json but claims "
|
|
||||||
"External-state mutations: none"
|
|
||||||
)
|
|
||||||
elif activity["seed_or_restore"] and not disclosed:
|
|
||||||
reasons.append(
|
|
||||||
"report mentions issue-lock file activity but External-state mutations "
|
|
||||||
"does not disclose read/write of gitea_issue_lock.json"
|
|
||||||
)
|
|
||||||
|
|
||||||
if activity["remove"]:
|
|
||||||
if _EXTERNAL_NONE_RE.search(text):
|
|
||||||
reasons.append(
|
|
||||||
"report mentions removing gitea_issue_lock.json but claims "
|
|
||||||
"External-state mutations: none"
|
|
||||||
)
|
|
||||||
elif not disclosed and _CLEANUP_ONLY_RE.search(text):
|
|
||||||
reasons.append(
|
|
||||||
"report removes issue lock but classifies it as cleanup only; "
|
|
||||||
"record under External-state mutations"
|
|
||||||
)
|
|
||||||
elif not disclosed:
|
|
||||||
reasons.append(
|
|
||||||
"report mentions deleting issue lock without External-state "
|
|
||||||
"mutation disclosure"
|
|
||||||
)
|
|
||||||
|
|
||||||
proven = not reasons
|
|
||||||
return {
|
|
||||||
"proven": proven,
|
|
||||||
"block": not proven,
|
|
||||||
"reasons": reasons,
|
|
||||||
"activity": activity,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def assess_manual_lock_pr_without_override(report_text: str) -> dict:
|
|
||||||
"""Block reports that created a PR via manual lock seed without override proof."""
|
|
||||||
text = report_text or ""
|
|
||||||
seeded = bool(_LOCK_SEED_RE.search(text))
|
|
||||||
created = bool(_PR_CREATED_RE.search(text))
|
|
||||||
if not (seeded and created):
|
|
||||||
return {"proven": True, "block": False, "reasons": []}
|
|
||||||
|
|
||||||
if _OVERRIDE_PROOF_RE.search(text):
|
|
||||||
return {"proven": True, "block": False, "reasons": []}
|
|
||||||
|
|
||||||
return {
|
|
||||||
"proven": False,
|
|
||||||
"block": True,
|
|
||||||
"reasons": [
|
|
||||||
"report created/opened a PR after manual issue-lock seeding without "
|
|
||||||
"operator override proof"
|
|
||||||
],
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def assess_author_reviewer_same_run_report(report_text: str) -> dict:
|
|
||||||
"""Reviewer handoff must not create and approve the same PR in one run (#447)."""
|
|
||||||
text = report_text or ""
|
|
||||||
if not (_PR_CREATED_RE.search(text) and _REVIEW_APPROVE_RE.search(text)):
|
|
||||||
return {"proven": True, "block": False, "reasons": []}
|
|
||||||
return {
|
|
||||||
"proven": False,
|
|
||||||
"block": True,
|
|
||||||
"reasons": [
|
|
||||||
"report mixes author-side PR creation and reviewer approval in one "
|
|
||||||
"final handoff; split author and reviewer sessions"
|
|
||||||
],
|
|
||||||
}
|
|
||||||
@@ -1,385 +0,0 @@
|
|||||||
"""Keyed, persistent issue-lock storage (#443).
|
|
||||||
|
|
||||||
Replaces the single global ``/tmp/gitea_issue_lock.json`` slot with per-issue
|
|
||||||
lock files under ``GITEA_ISSUE_LOCK_DIR`` (default
|
|
||||||
``~/.cache/gitea-tools/issue-locks``). Each MCP session binds its active lock
|
|
||||||
via a per-process pointer file so concurrent repos/issues never clobber each
|
|
||||||
other.
|
|
||||||
"""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import json
|
|
||||||
import os
|
|
||||||
import re
|
|
||||||
import tempfile
|
|
||||||
from datetime import datetime, timedelta, timezone
|
|
||||||
from typing import Any
|
|
||||||
|
|
||||||
LOCK_DIR_ENV = "GITEA_ISSUE_LOCK_DIR"
|
|
||||||
DEFAULT_LOCK_DIR = os.path.expanduser("~/.cache/gitea-tools/issue-locks")
|
|
||||||
WORK_LEASE_TTL_HOURS = 4
|
|
||||||
AUTHOR_ISSUE_WORK_LEASE = "author_issue_work"
|
|
||||||
|
|
||||||
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
|
|
||||||
|
|
||||||
|
|
||||||
def default_lock_dir() -> str:
|
|
||||||
raw = (os.environ.get(LOCK_DIR_ENV) or DEFAULT_LOCK_DIR).strip()
|
|
||||||
return raw or DEFAULT_LOCK_DIR
|
|
||||||
|
|
||||||
|
|
||||||
def _sanitize_segment(value: str) -> str:
|
|
||||||
text = (value or "").strip()
|
|
||||||
if not text:
|
|
||||||
return "_"
|
|
||||||
return _SAFE_SEGMENT_RE.sub("_", text)
|
|
||||||
|
|
||||||
|
|
||||||
def lock_key(
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
org: str,
|
|
||||||
repo: str,
|
|
||||||
issue_number: int,
|
|
||||||
) -> str:
|
|
||||||
return "-".join(
|
|
||||||
_sanitize_segment(part)
|
|
||||||
for part in (remote, org, repo, str(issue_number))
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def lock_file_path(
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
org: str,
|
|
||||||
repo: str,
|
|
||||||
issue_number: int,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> str:
|
|
||||||
root = (lock_dir or default_lock_dir()).strip()
|
|
||||||
return os.path.join(root, f"{lock_key(remote=remote, org=org, repo=repo, issue_number=issue_number)}.json")
|
|
||||||
|
|
||||||
|
|
||||||
def session_pointer_path(lock_dir: str | None = None) -> str:
|
|
||||||
root = (lock_dir or default_lock_dir()).strip()
|
|
||||||
return os.path.join(root, f"session-{os.getpid()}.json")
|
|
||||||
|
|
||||||
|
|
||||||
def _ensure_lock_dir(lock_dir: str | None = None) -> str:
|
|
||||||
root = (lock_dir or default_lock_dir()).strip()
|
|
||||||
os.makedirs(root, mode=0o700, exist_ok=True)
|
|
||||||
return root
|
|
||||||
|
|
||||||
|
|
||||||
def read_lock_file(path: str) -> dict[str, Any] | None:
|
|
||||||
lock_path = (path or "").strip()
|
|
||||||
if not lock_path or not os.path.exists(lock_path):
|
|
||||||
return None
|
|
||||||
try:
|
|
||||||
with open(lock_path, encoding="utf-8") as handle:
|
|
||||||
data = json.load(handle)
|
|
||||||
except (OSError, json.JSONDecodeError):
|
|
||||||
return None
|
|
||||||
return data if isinstance(data, dict) else None
|
|
||||||
|
|
||||||
|
|
||||||
def save_lock_file(path: str, data: dict[str, Any]) -> None:
|
|
||||||
lock_path = (path or "").strip()
|
|
||||||
if not lock_path:
|
|
||||||
raise ValueError("lock path is required (fail closed)")
|
|
||||||
parent = os.path.dirname(lock_path) or "."
|
|
||||||
os.makedirs(parent, mode=0o700, exist_ok=True)
|
|
||||||
payload = json.dumps(data, indent=2, sort_keys=True) + "\n"
|
|
||||||
fd, temp_path = tempfile.mkstemp(prefix=".lock-", suffix=".json", dir=parent)
|
|
||||||
try:
|
|
||||||
with os.fdopen(fd, "w", encoding="utf-8") as handle:
|
|
||||||
handle.write(payload)
|
|
||||||
handle.flush()
|
|
||||||
os.fsync(handle.fileno())
|
|
||||||
os.replace(temp_path, lock_path)
|
|
||||||
finally:
|
|
||||||
if os.path.exists(temp_path):
|
|
||||||
try:
|
|
||||||
os.remove(temp_path)
|
|
||||||
except OSError:
|
|
||||||
pass
|
|
||||||
|
|
||||||
|
|
||||||
def bind_session_lock(lock_data: dict[str, Any], lock_dir: str | None = None) -> str:
|
|
||||||
"""Persist a keyed lock and bind it to the current process session."""
|
|
||||||
remote = str(lock_data.get("remote") or "")
|
|
||||||
org = str(lock_data.get("org") or "")
|
|
||||||
repo = str(lock_data.get("repo") or "")
|
|
||||||
issue_number = int(lock_data.get("issue_number") or 0)
|
|
||||||
if not remote or not org or not repo or issue_number <= 0:
|
|
||||||
raise ValueError("lock record must include remote, org, repo, and issue_number")
|
|
||||||
|
|
||||||
root = _ensure_lock_dir(lock_dir)
|
|
||||||
path = lock_file_path(
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
issue_number=issue_number,
|
|
||||||
lock_dir=root,
|
|
||||||
)
|
|
||||||
record = dict(lock_data)
|
|
||||||
record["lock_file_path"] = path
|
|
||||||
record["session_pid"] = os.getpid()
|
|
||||||
save_lock_file(path, record)
|
|
||||||
|
|
||||||
pointer = {
|
|
||||||
"pid": os.getpid(),
|
|
||||||
"lock_file_path": path,
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"branch_name": record.get("branch_name"),
|
|
||||||
"remote": remote,
|
|
||||||
"org": org,
|
|
||||||
"repo": repo,
|
|
||||||
}
|
|
||||||
save_lock_file(session_pointer_path(root), pointer)
|
|
||||||
return path
|
|
||||||
|
|
||||||
|
|
||||||
def read_session_issue_lock(lock_dir: str | None = None) -> dict[str, Any] | None:
|
|
||||||
root = (lock_dir or default_lock_dir()).strip()
|
|
||||||
pointer = read_lock_file(session_pointer_path(root))
|
|
||||||
if not pointer:
|
|
||||||
return None
|
|
||||||
lock_path = str(pointer.get("lock_file_path") or "").strip()
|
|
||||||
if not lock_path:
|
|
||||||
return None
|
|
||||||
return read_lock_file(lock_path)
|
|
||||||
|
|
||||||
|
|
||||||
def load_issue_lock(
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
org: str,
|
|
||||||
repo: str,
|
|
||||||
issue_number: int,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
return read_lock_file(
|
|
||||||
lock_file_path(
|
|
||||||
remote=remote,
|
|
||||||
org=org,
|
|
||||||
repo=repo,
|
|
||||||
issue_number=issue_number,
|
|
||||||
lock_dir=lock_dir,
|
|
||||||
)
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def iter_lock_files(lock_dir: str | None = None) -> list[str]:
|
|
||||||
root = (lock_dir or default_lock_dir()).strip()
|
|
||||||
if not os.path.isdir(root):
|
|
||||||
return []
|
|
||||||
paths: list[str] = []
|
|
||||||
for name in os.listdir(root):
|
|
||||||
if not name.endswith(".json") or name.startswith("session-"):
|
|
||||||
continue
|
|
||||||
paths.append(os.path.join(root, name))
|
|
||||||
return sorted(paths)
|
|
||||||
|
|
||||||
|
|
||||||
def find_lock_for_branch(
|
|
||||||
*,
|
|
||||||
remote: str,
|
|
||||||
org: str,
|
|
||||||
repo: str,
|
|
||||||
branch_name: str,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
target = (branch_name or "").strip()
|
|
||||||
if not target:
|
|
||||||
return None
|
|
||||||
for path in iter_lock_files(lock_dir):
|
|
||||||
lock = read_lock_file(path)
|
|
||||||
if not lock:
|
|
||||||
continue
|
|
||||||
if (
|
|
||||||
str(lock.get("remote") or "") == remote
|
|
||||||
and str(lock.get("org") or "") == org
|
|
||||||
and str(lock.get("repo") or "") == repo
|
|
||||||
and str(lock.get("branch_name") or "").strip() == target
|
|
||||||
):
|
|
||||||
lock = dict(lock)
|
|
||||||
lock.setdefault("lock_file_path", path)
|
|
||||||
return lock
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def _lease_now(now: datetime | None = None) -> datetime:
|
|
||||||
return now or datetime.now(timezone.utc)
|
|
||||||
|
|
||||||
|
|
||||||
def _parse_lease_timestamp(value: str | None) -> datetime | None:
|
|
||||||
text = (value or "").strip()
|
|
||||||
if not text:
|
|
||||||
return None
|
|
||||||
try:
|
|
||||||
return datetime.fromisoformat(text.replace("Z", "+00:00")).astimezone(timezone.utc)
|
|
||||||
except ValueError:
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def lease_expires_at(lock: dict[str, Any] | None) -> datetime | None:
|
|
||||||
if not lock:
|
|
||||||
return None
|
|
||||||
lease = lock.get("work_lease")
|
|
||||||
if not isinstance(lease, dict):
|
|
||||||
return None
|
|
||||||
return _parse_lease_timestamp(lease.get("expires_at"))
|
|
||||||
|
|
||||||
|
|
||||||
def is_lease_expired(lock: dict[str, Any] | None, *, now: datetime | None = None) -> bool:
|
|
||||||
expires = lease_expires_at(lock)
|
|
||||||
if expires is None:
|
|
||||||
return False
|
|
||||||
return expires <= _lease_now(now)
|
|
||||||
|
|
||||||
|
|
||||||
def is_lease_live(lock: dict[str, Any] | None, *, now: datetime | None = None) -> bool:
|
|
||||||
if not lock:
|
|
||||||
return False
|
|
||||||
lease = lock.get("work_lease")
|
|
||||||
if not isinstance(lease, dict):
|
|
||||||
return True
|
|
||||||
expires = _parse_lease_timestamp(lease.get("expires_at"))
|
|
||||||
if expires is None:
|
|
||||||
return True
|
|
||||||
return expires > _lease_now(now)
|
|
||||||
|
|
||||||
|
|
||||||
def _same_realpath(left: str | None, right: str | None) -> bool:
|
|
||||||
if not left or not right:
|
|
||||||
return False
|
|
||||||
try:
|
|
||||||
return os.path.realpath(left) == os.path.realpath(right)
|
|
||||||
except OSError:
|
|
||||||
return left == right
|
|
||||||
|
|
||||||
|
|
||||||
def assess_same_issue_lease_conflict(
|
|
||||||
existing_lock: dict[str, Any] | None,
|
|
||||||
*,
|
|
||||||
issue_number: int,
|
|
||||||
branch_name: str,
|
|
||||||
worktree_path: str,
|
|
||||||
operation_type: str = AUTHOR_ISSUE_WORK_LEASE,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> str | None:
|
|
||||||
"""Return a fail-closed error when a competing live lease blocks acquisition."""
|
|
||||||
if not existing_lock:
|
|
||||||
return None
|
|
||||||
|
|
||||||
existing_issue = existing_lock.get("issue_number")
|
|
||||||
lease = existing_lock.get("work_lease")
|
|
||||||
existing_operation = (
|
|
||||||
lease.get("operation_type")
|
|
||||||
if isinstance(lease, dict)
|
|
||||||
else AUTHOR_ISSUE_WORK_LEASE
|
|
||||||
)
|
|
||||||
if existing_issue != issue_number or existing_operation != operation_type:
|
|
||||||
return None
|
|
||||||
|
|
||||||
existing_branch = existing_lock.get("branch_name")
|
|
||||||
existing_worktree = existing_lock.get("worktree_path")
|
|
||||||
same_owner = (
|
|
||||||
existing_branch == branch_name
|
|
||||||
and _same_realpath(str(existing_worktree or ""), worktree_path)
|
|
||||||
)
|
|
||||||
if is_lease_expired(existing_lock, now=now):
|
|
||||||
return (
|
|
||||||
f"Issue #{issue_number} has an expired {operation_type} lease on "
|
|
||||||
f"branch '{existing_branch}' from worktree '{existing_worktree}'. "
|
|
||||||
"Recovery review is required before takeover (fail closed)"
|
|
||||||
)
|
|
||||||
if same_owner:
|
|
||||||
return None
|
|
||||||
return (
|
|
||||||
f"Issue #{issue_number} already has an active {operation_type} lease on "
|
|
||||||
f"branch '{existing_branch}' from worktree '{existing_worktree}' "
|
|
||||||
"(fail closed)"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def assess_foreign_lock_overwrite(
|
|
||||||
existing_lock: dict[str, Any] | None,
|
|
||||||
incoming_lock: dict[str, Any],
|
|
||||||
*,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> str | None:
|
|
||||||
"""Block writes that would clobber an unrelated live lease on the same key."""
|
|
||||||
if not existing_lock:
|
|
||||||
return None
|
|
||||||
|
|
||||||
same_issue = existing_lock.get("issue_number") == incoming_lock.get("issue_number")
|
|
||||||
same_branch = existing_lock.get("branch_name") == incoming_lock.get("branch_name")
|
|
||||||
same_worktree = _same_realpath(
|
|
||||||
str(existing_lock.get("worktree_path") or ""),
|
|
||||||
str(incoming_lock.get("worktree_path") or ""),
|
|
||||||
)
|
|
||||||
if same_issue and same_branch and same_worktree:
|
|
||||||
return None
|
|
||||||
if not is_lease_live(existing_lock, now=now):
|
|
||||||
return None
|
|
||||||
return (
|
|
||||||
"Refusing to overwrite a live foreign issue lock "
|
|
||||||
f"(issue #{existing_lock.get('issue_number')}, "
|
|
||||||
f"branch '{existing_lock.get('branch_name')}', "
|
|
||||||
f"worktree '{existing_lock.get('worktree_path')}') (fail closed)"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def find_live_lock_for_branch(
|
|
||||||
branch_name: str,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
target = (branch_name or "").strip()
|
|
||||||
if not target:
|
|
||||||
return None
|
|
||||||
for path in iter_lock_files(lock_dir):
|
|
||||||
lock = read_lock_file(path)
|
|
||||||
if not lock:
|
|
||||||
continue
|
|
||||||
if str(lock.get("branch_name") or "").strip() != target:
|
|
||||||
continue
|
|
||||||
if not is_lease_live(lock):
|
|
||||||
continue
|
|
||||||
record = dict(lock)
|
|
||||||
record.setdefault("lock_file_path", path)
|
|
||||||
return record
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def resolve_locked_branch_for_session(
|
|
||||||
branch_name: str | None = None,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> str:
|
|
||||||
if branch_name:
|
|
||||||
lock = find_live_lock_for_branch(branch_name, lock_dir)
|
|
||||||
if lock:
|
|
||||||
return str(lock.get("branch_name") or "")
|
|
||||||
lock = read_session_issue_lock(lock_dir)
|
|
||||||
return str((lock or {}).get("branch_name") or "")
|
|
||||||
|
|
||||||
|
|
||||||
def has_active_issue_lock(
|
|
||||||
branch: str,
|
|
||||||
*,
|
|
||||||
lock_dir: str | None = None,
|
|
||||||
) -> bool:
|
|
||||||
target = (branch or "").strip()
|
|
||||||
if not target:
|
|
||||||
return False
|
|
||||||
for path in iter_lock_files(lock_dir):
|
|
||||||
lock = read_lock_file(path)
|
|
||||||
if not lock:
|
|
||||||
continue
|
|
||||||
if str(lock.get("branch_name") or "").strip() != target:
|
|
||||||
continue
|
|
||||||
if is_lease_live(lock):
|
|
||||||
return True
|
|
||||||
return False
|
|
||||||
@@ -1,180 +0,0 @@
|
|||||||
"""Early duplicate-work detection for author work-issue sessions (#400)."""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
from typing import Any
|
|
||||||
|
|
||||||
import issue_claim_heartbeat as claim_hb
|
|
||||||
|
|
||||||
PHASE_LOCK = "lock_issue"
|
|
||||||
PHASE_COMMIT = "commit"
|
|
||||||
PHASE_PUSH = "push"
|
|
||||||
PHASE_CREATE_PR = "create_pr"
|
|
||||||
|
|
||||||
OUTCOME_DUPLICATE_PR_PREVENTED = "duplicate_pr_prevented"
|
|
||||||
OUTCOME_DUPLICATE_BRANCH_PREVENTED = "duplicate_branch_prevented"
|
|
||||||
OUTCOME_DUPLICATE_COMMIT_PREVENTED = "duplicate_commit_prevented"
|
|
||||||
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED = "duplicate_work_not_prevented"
|
|
||||||
|
|
||||||
_ACTIVE_CLAIM_STATUSES = frozenset({"active", "awaiting_review"})
|
|
||||||
|
|
||||||
|
|
||||||
def _issue_pattern(issue_number: int) -> str:
|
|
||||||
return f"issue-{int(issue_number)}"
|
|
||||||
|
|
||||||
|
|
||||||
def _linked_open_pr(issue_number: int, open_prs: list[dict]) -> dict | None:
|
|
||||||
return claim_hb._linked_open_pr(issue_number, open_prs)
|
|
||||||
|
|
||||||
|
|
||||||
def _matching_branches(
|
|
||||||
issue_number: int,
|
|
||||||
branch_names: list[str],
|
|
||||||
*,
|
|
||||||
locked_branch: str | None = None,
|
|
||||||
) -> list[str]:
|
|
||||||
pattern = _issue_pattern(issue_number)
|
|
||||||
matches = [
|
|
||||||
name for name in (branch_names or [])
|
|
||||||
if pattern in (name or "").lower()
|
|
||||||
]
|
|
||||||
if locked_branch:
|
|
||||||
locked = locked_branch.strip()
|
|
||||||
matches = [name for name in matches if name != locked]
|
|
||||||
return matches
|
|
||||||
|
|
||||||
|
|
||||||
def assess_work_issue_duplicate_gate(
|
|
||||||
issue_number: int,
|
|
||||||
*,
|
|
||||||
open_prs: list[dict] | None = None,
|
|
||||||
branch_names: list[str] | None = None,
|
|
||||||
claim_entry: dict | None = None,
|
|
||||||
locked_branch: str | None = None,
|
|
||||||
phase: str = PHASE_LOCK,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Fail closed when duplicate work is already in flight for an issue."""
|
|
||||||
reasons: list[str] = []
|
|
||||||
outcome = OUTCOME_DUPLICATE_WORK_NOT_PREVENTED
|
|
||||||
prs = list(open_prs or [])
|
|
||||||
branches = list(branch_names or [])
|
|
||||||
pattern = _issue_pattern(issue_number)
|
|
||||||
|
|
||||||
linked = _linked_open_pr(issue_number, prs)
|
|
||||||
if linked:
|
|
||||||
reasons.append(
|
|
||||||
f"open PR #{linked.get('number')} already covers issue "
|
|
||||||
f"#{issue_number} (fail closed)"
|
|
||||||
)
|
|
||||||
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
|
|
||||||
|
|
||||||
conflicting_branches = _matching_branches(
|
|
||||||
issue_number, branches, locked_branch=locked_branch
|
|
||||||
)
|
|
||||||
if conflicting_branches:
|
|
||||||
names = ", ".join(conflicting_branches[:5])
|
|
||||||
reasons.append(
|
|
||||||
f"remote branch(es) already match issue pattern '{pattern}': "
|
|
||||||
f"{names} (fail closed)"
|
|
||||||
)
|
|
||||||
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
||||||
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
||||||
|
|
||||||
entry = claim_entry or {}
|
|
||||||
if entry.get("linked_open_pr") and not linked:
|
|
||||||
reasons.append(
|
|
||||||
f"claim inventory reports open PR #{entry['linked_open_pr']} "
|
|
||||||
f"for issue #{issue_number} (fail closed)"
|
|
||||||
)
|
|
||||||
outcome = OUTCOME_DUPLICATE_PR_PREVENTED
|
|
||||||
|
|
||||||
status = (entry.get("status") or "").strip().lower()
|
|
||||||
if status in _ACTIVE_CLAIM_STATUSES and not linked:
|
|
||||||
heartbeat = entry.get("latest_heartbeat") or {}
|
|
||||||
claim_branch = (heartbeat.get("branch") or "").strip()
|
|
||||||
if locked_branch and claim_branch and claim_branch != locked_branch:
|
|
||||||
reasons.append(
|
|
||||||
f"active claim lease on branch '{claim_branch}' blocks "
|
|
||||||
f"work on '{locked_branch}' for issue #{issue_number} "
|
|
||||||
"(fail closed)"
|
|
||||||
)
|
|
||||||
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
||||||
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
||||||
elif not locked_branch and status == "active":
|
|
||||||
reasons.append(
|
|
||||||
f"issue #{issue_number} has an active claim lease "
|
|
||||||
"(fail closed)"
|
|
||||||
)
|
|
||||||
if outcome == OUTCOME_DUPLICATE_WORK_NOT_PREVENTED:
|
|
||||||
outcome = OUTCOME_DUPLICATE_BRANCH_PREVENTED
|
|
||||||
|
|
||||||
if phase in {PHASE_COMMIT, PHASE_PUSH} and reasons:
|
|
||||||
if outcome == OUTCOME_DUPLICATE_PR_PREVENTED:
|
|
||||||
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
|
|
||||||
elif outcome == OUTCOME_DUPLICATE_BRANCH_PREVENTED:
|
|
||||||
outcome = OUTCOME_DUPLICATE_COMMIT_PREVENTED
|
|
||||||
|
|
||||||
block = bool(reasons)
|
|
||||||
return {
|
|
||||||
"block": block,
|
|
||||||
"performed": not block,
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"phase": phase,
|
|
||||||
"outcome": outcome,
|
|
||||||
"linked_open_pr": linked.get("number") if linked else entry.get("linked_open_pr"),
|
|
||||||
"conflicting_branches": conflicting_branches,
|
|
||||||
"claim_status": status or None,
|
|
||||||
"reasons": reasons,
|
|
||||||
"safe_next_action": (
|
|
||||||
"stop before mutating; preserve local work and produce a "
|
|
||||||
"reconciliation handoff if a concurrent PR appeared after push"
|
|
||||||
if block and phase == PHASE_CREATE_PR
|
|
||||||
else "stop before mutating; do not commit or push duplicate work"
|
|
||||||
if block
|
|
||||||
else "proceed"
|
|
||||||
),
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def assess_work_issue_duplicate_report(report_text: str) -> dict[str, Any]:
|
|
||||||
"""Require explicit duplicate-work outcome wording in work-issue reports."""
|
|
||||||
text = (report_text or "").lower()
|
|
||||||
markers = {
|
|
||||||
OUTCOME_DUPLICATE_PR_PREVENTED: (
|
|
||||||
"duplicate pr prevented",
|
|
||||||
"duplicate_pr_prevented",
|
|
||||||
),
|
|
||||||
OUTCOME_DUPLICATE_BRANCH_PREVENTED: (
|
|
||||||
"duplicate branch prevented",
|
|
||||||
"duplicate_branch_prevented",
|
|
||||||
),
|
|
||||||
OUTCOME_DUPLICATE_COMMIT_PREVENTED: (
|
|
||||||
"duplicate commit prevented",
|
|
||||||
"duplicate_commit_prevented",
|
|
||||||
),
|
|
||||||
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED: (
|
|
||||||
"duplicate work not prevented",
|
|
||||||
"duplicate_work_not_prevented",
|
|
||||||
"no duplicate work",
|
|
||||||
),
|
|
||||||
}
|
|
||||||
matched = [
|
|
||||||
key for key, phrases in markers.items()
|
|
||||||
if any(phrase in text for phrase in phrases)
|
|
||||||
]
|
|
||||||
if len(matched) != 1:
|
|
||||||
return {
|
|
||||||
"complete": False,
|
|
||||||
"downgraded": True,
|
|
||||||
"reasons": [
|
|
||||||
"work-issue report must state exactly one duplicate-work "
|
|
||||||
"outcome (duplicate PR/branch/commit prevented, or "
|
|
||||||
"duplicate work not prevented)"
|
|
||||||
],
|
|
||||||
}
|
|
||||||
return {
|
|
||||||
"complete": True,
|
|
||||||
"downgraded": False,
|
|
||||||
"outcome": matched[0],
|
|
||||||
"reasons": [],
|
|
||||||
}
|
|
||||||
@@ -13,9 +13,9 @@ import subprocess
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from reviewer_worktree import parse_dirty_tracked_files
|
from reviewer_worktree import parse_dirty_tracked_files
|
||||||
import issue_lock_store
|
|
||||||
|
|
||||||
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
|
||||||
|
ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")
|
||||||
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
CLOSES_FIXES_RE = re.compile(r"\b(?:closes|fixes)\s+#(\d+)\b", re.IGNORECASE)
|
||||||
|
|
||||||
|
|
||||||
@@ -37,18 +37,22 @@ def resolve_worktree_path(project_root: str, branch: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
|
def read_issue_lock(path: str | None = None) -> dict[str, Any] | None:
|
||||||
if path:
|
lock_path = (path or ISSUE_LOCK_FILE).strip()
|
||||||
return issue_lock_store.read_lock_file(path.strip())
|
if not lock_path or not os.path.exists(lock_path):
|
||||||
return issue_lock_store.read_session_issue_lock()
|
return None
|
||||||
|
try:
|
||||||
|
with open(lock_path, encoding="utf-8") as handle:
|
||||||
|
data = json.load(handle)
|
||||||
|
except (OSError, json.JSONDecodeError):
|
||||||
|
return None
|
||||||
|
return data if isinstance(data, dict) else None
|
||||||
|
|
||||||
|
|
||||||
def has_active_issue_lock(branch: str, lock_path: str | None = None) -> bool:
|
def has_active_issue_lock(branch: str, lock_path: str | None = None) -> bool:
|
||||||
if lock_path:
|
lock = read_issue_lock(lock_path)
|
||||||
lock = issue_lock_store.read_lock_file(lock_path.strip())
|
|
||||||
if not lock:
|
if not lock:
|
||||||
return False
|
return False
|
||||||
return (lock.get("branch_name") or "").strip() == (branch or "").strip()
|
return (lock.get("branch_name") or "").strip() == (branch or "").strip()
|
||||||
return issue_lock_store.has_active_issue_lock(branch)
|
|
||||||
|
|
||||||
|
|
||||||
def collect_open_pr_heads(open_prs: list[dict[str, Any]]) -> set[str]:
|
def collect_open_pr_heads(open_prs: list[dict[str, Any]]) -> set[str]:
|
||||||
|
|||||||
@@ -3624,12 +3624,9 @@ def assess_work_issue_mode_isolation(report_text: str) -> dict:
|
|||||||
|
|
||||||
def assess_work_issue_final_report(report_text: str) -> dict:
|
def assess_work_issue_final_report(report_text: str) -> dict:
|
||||||
"""#139: composite verifier for work-issue final reports."""
|
"""#139: composite verifier for work-issue final reports."""
|
||||||
from issue_work_duplicate_gate import assess_work_issue_duplicate_report
|
|
||||||
|
|
||||||
checks = {
|
checks = {
|
||||||
"workflow_source": assess_work_issue_workflow_source(report_text),
|
"workflow_source": assess_work_issue_workflow_source(report_text),
|
||||||
"mode_isolation": assess_work_issue_mode_isolation(report_text),
|
"mode_isolation": assess_work_issue_mode_isolation(report_text),
|
||||||
"duplicate_work_outcome": assess_work_issue_duplicate_report(report_text),
|
|
||||||
}
|
}
|
||||||
|
|
||||||
reasons = []
|
reasons = []
|
||||||
|
|||||||
@@ -1,382 +0,0 @@
|
|||||||
"""Per-PR reviewer leases for safe parallel review sessions (#407)."""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import os
|
|
||||||
import re
|
|
||||||
import uuid
|
|
||||||
from datetime import datetime, timedelta, timezone
|
|
||||||
from typing import Any
|
|
||||||
|
|
||||||
MARKER = "<!-- mcp-review-lease:v1 -->"
|
|
||||||
|
|
||||||
_FIELD_RE = re.compile(
|
|
||||||
r"^\s*([a-z_]+)\s*:\s*(.+?)\s*$",
|
|
||||||
re.IGNORECASE | re.MULTILINE,
|
|
||||||
)
|
|
||||||
_FULL_SHA = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
|
|
||||||
|
|
||||||
_TERMINAL_PHASES = frozenset({"done", "released", "blocked"})
|
|
||||||
_ACTIVE_PHASES = frozenset({
|
|
||||||
"claimed",
|
|
||||||
"validating",
|
|
||||||
"approved",
|
|
||||||
"request-changes",
|
|
||||||
"merging",
|
|
||||||
})
|
|
||||||
|
|
||||||
DEFAULT_LEASE_TTL_MINUTES = 120
|
|
||||||
STALE_WARNING_MINUTES = 30
|
|
||||||
RECLAIMABLE_MINUTES = 60
|
|
||||||
|
|
||||||
_SESSION_LEASE: dict[str, Any] | None = None
|
|
||||||
|
|
||||||
|
|
||||||
def _parse_timestamp(value: str | None) -> datetime | None:
|
|
||||||
if not value:
|
|
||||||
return None
|
|
||||||
text = value.strip()
|
|
||||||
if text.endswith("Z"):
|
|
||||||
text = text[:-1] + "+00:00"
|
|
||||||
try:
|
|
||||||
parsed = datetime.fromisoformat(text)
|
|
||||||
except ValueError:
|
|
||||||
return None
|
|
||||||
if parsed.tzinfo is None:
|
|
||||||
return parsed.replace(tzinfo=timezone.utc)
|
|
||||||
return parsed.astimezone(timezone.utc)
|
|
||||||
|
|
||||||
|
|
||||||
def _normalize_sha(value: str | None) -> str | None:
|
|
||||||
text = (value or "").strip().lower()
|
|
||||||
return text if text and _FULL_SHA.match(text) else None
|
|
||||||
|
|
||||||
|
|
||||||
def _parse_pr_ref(value: str | None) -> int | None:
|
|
||||||
digits = re.sub(r"[^\d]", "", value or "")
|
|
||||||
return int(digits) if digits.isdigit() else None
|
|
||||||
|
|
||||||
|
|
||||||
def new_session_id() -> str:
|
|
||||||
return f"{os.getpid()}-{uuid.uuid4().hex[:12]}"
|
|
||||||
|
|
||||||
|
|
||||||
def format_lease_body(
|
|
||||||
*,
|
|
||||||
repo: str,
|
|
||||||
pr_number: int,
|
|
||||||
issue_number: int | None,
|
|
||||||
reviewer_identity: str,
|
|
||||||
profile: str,
|
|
||||||
session_id: str,
|
|
||||||
worktree: str,
|
|
||||||
phase: str,
|
|
||||||
candidate_head: str | None,
|
|
||||||
target_branch: str,
|
|
||||||
target_branch_sha: str | None,
|
|
||||||
last_activity: datetime | None = None,
|
|
||||||
expires_at: datetime | None = None,
|
|
||||||
blocker: str = "none",
|
|
||||||
) -> str:
|
|
||||||
now = last_activity or datetime.now(timezone.utc)
|
|
||||||
expires = expires_at or (now + timedelta(minutes=DEFAULT_LEASE_TTL_MINUTES))
|
|
||||||
last_text = now.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
|
|
||||||
"+00:00", "Z"
|
|
||||||
)
|
|
||||||
expires_text = expires.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace(
|
|
||||||
"+00:00", "Z"
|
|
||||||
)
|
|
||||||
issue_text = f"#{issue_number}" if issue_number else "none"
|
|
||||||
lines = [
|
|
||||||
MARKER,
|
|
||||||
f"repo: {repo}",
|
|
||||||
f"pr: #{pr_number}",
|
|
||||||
f"issue: {issue_text}",
|
|
||||||
f"reviewer_identity: {reviewer_identity}",
|
|
||||||
f"profile: {profile}",
|
|
||||||
f"session_id: {session_id}",
|
|
||||||
f"worktree: {worktree}",
|
|
||||||
f"phase: {phase}",
|
|
||||||
f"candidate_head: {candidate_head or 'none'}",
|
|
||||||
f"target_branch: {target_branch}",
|
|
||||||
f"target_branch_sha: {target_branch_sha or 'none'}",
|
|
||||||
f"last_activity: {last_text}",
|
|
||||||
f"expires_at: {expires_text}",
|
|
||||||
f"blocker: {blocker}",
|
|
||||||
]
|
|
||||||
return "\n".join(lines)
|
|
||||||
|
|
||||||
|
|
||||||
def parse_lease_comment(body: str) -> dict[str, Any] | None:
|
|
||||||
text = body or ""
|
|
||||||
if MARKER not in text:
|
|
||||||
return None
|
|
||||||
fields: dict[str, str] = {}
|
|
||||||
for match in _FIELD_RE.finditer(text):
|
|
||||||
fields[match.group(1).strip().lower()] = match.group(2).strip()
|
|
||||||
if not fields:
|
|
||||||
return None
|
|
||||||
return {
|
|
||||||
"repo": fields.get("repo"),
|
|
||||||
"pr_number": _parse_pr_ref(fields.get("pr")),
|
|
||||||
"issue_number": _parse_pr_ref(fields.get("issue")),
|
|
||||||
"reviewer_identity": fields.get("reviewer_identity"),
|
|
||||||
"profile": fields.get("profile"),
|
|
||||||
"session_id": fields.get("session_id"),
|
|
||||||
"worktree": fields.get("worktree"),
|
|
||||||
"phase": (fields.get("phase") or "").strip().lower() or None,
|
|
||||||
"candidate_head": _normalize_sha(fields.get("candidate_head")),
|
|
||||||
"target_branch": fields.get("target_branch"),
|
|
||||||
"target_branch_sha": _normalize_sha(fields.get("target_branch_sha")),
|
|
||||||
"last_activity": fields.get("last_activity"),
|
|
||||||
"expires_at": fields.get("expires_at"),
|
|
||||||
"blocker": fields.get("blocker"),
|
|
||||||
"raw_fields": fields,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def _lease_entries(comments: list[dict], *, pr_number: int) -> list[dict]:
|
|
||||||
entries: list[dict] = []
|
|
||||||
for comment in comments or []:
|
|
||||||
parsed = parse_lease_comment(comment.get("body") or "")
|
|
||||||
if not parsed:
|
|
||||||
continue
|
|
||||||
if parsed.get("pr_number") not in (None, pr_number):
|
|
||||||
continue
|
|
||||||
entries.append({
|
|
||||||
**parsed,
|
|
||||||
"comment_id": comment.get("id"),
|
|
||||||
"author": (comment.get("user") or {}).get("login") or comment.get("author"),
|
|
||||||
"created_at": comment.get("created_at"),
|
|
||||||
"updated_at": comment.get("updated_at"),
|
|
||||||
})
|
|
||||||
return entries
|
|
||||||
|
|
||||||
|
|
||||||
def _lease_expired(lease: dict, *, now: datetime) -> bool:
|
|
||||||
expires_at = _parse_timestamp(lease.get("expires_at"))
|
|
||||||
return bool(expires_at and expires_at <= now)
|
|
||||||
|
|
||||||
|
|
||||||
def _minutes_since_activity(lease: dict, *, now: datetime) -> float | None:
|
|
||||||
last = _parse_timestamp(lease.get("last_activity"))
|
|
||||||
if not last:
|
|
||||||
return None
|
|
||||||
return (now - last).total_seconds() / 60.0
|
|
||||||
|
|
||||||
|
|
||||||
def classify_lease_freshness(lease: dict, *, now: datetime | None = None) -> str:
|
|
||||||
"""Return active, stale_warning, reclaimable, expired, or terminal."""
|
|
||||||
now = now or datetime.now(timezone.utc)
|
|
||||||
phase = (lease.get("phase") or "").strip().lower()
|
|
||||||
if phase in _TERMINAL_PHASES:
|
|
||||||
return "terminal"
|
|
||||||
if _lease_expired(lease, now=now):
|
|
||||||
return "expired"
|
|
||||||
minutes = _minutes_since_activity(lease, now=now)
|
|
||||||
if minutes is None:
|
|
||||||
return "active"
|
|
||||||
if minutes >= RECLAIMABLE_MINUTES:
|
|
||||||
return "reclaimable"
|
|
||||||
if minutes >= STALE_WARNING_MINUTES:
|
|
||||||
return "stale_warning"
|
|
||||||
return "active"
|
|
||||||
|
|
||||||
|
|
||||||
def find_active_reviewer_lease(
|
|
||||||
comments: list[dict],
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> dict[str, Any] | None:
|
|
||||||
"""Newest non-terminal, unexpired lease for *pr_number*."""
|
|
||||||
now = now or datetime.now(timezone.utc)
|
|
||||||
for lease in reversed(_lease_entries(comments, pr_number=pr_number)):
|
|
||||||
phase = (lease.get("phase") or "").strip().lower()
|
|
||||||
if phase in _TERMINAL_PHASES:
|
|
||||||
continue
|
|
||||||
if _lease_expired(lease, now=now):
|
|
||||||
continue
|
|
||||||
if phase in _ACTIVE_PHASES or phase:
|
|
||||||
lease = dict(lease)
|
|
||||||
lease["freshness"] = classify_lease_freshness(lease, now=now)
|
|
||||||
return lease
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def assess_acquire_lease(
|
|
||||||
comments: list[dict],
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
reviewer_identity: str,
|
|
||||||
profile: str,
|
|
||||||
session_id: str,
|
|
||||||
repo: str,
|
|
||||||
issue_number: int | None,
|
|
||||||
worktree: str,
|
|
||||||
candidate_head: str | None,
|
|
||||||
target_branch: str,
|
|
||||||
target_branch_sha: str | None,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Fail closed when another session holds an active lease."""
|
|
||||||
now = now or datetime.now(timezone.utc)
|
|
||||||
reasons: list[str] = []
|
|
||||||
existing = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
|
|
||||||
if existing:
|
|
||||||
owner_session = (existing.get("session_id") or "").strip()
|
|
||||||
freshness = existing.get("freshness") or classify_lease_freshness(existing, now=now)
|
|
||||||
if owner_session and owner_session != session_id and freshness in {
|
|
||||||
"active", "stale_warning"
|
|
||||||
}:
|
|
||||||
reasons.append(
|
|
||||||
f"PR #{pr_number} already has active reviewer lease "
|
|
||||||
f"(session_id={owner_session}, phase={existing.get('phase')})"
|
|
||||||
)
|
|
||||||
elif owner_session and owner_session != session_id and freshness == "reclaimable":
|
|
||||||
reasons.append(
|
|
||||||
f"PR #{pr_number} lease is reclaimable but still held by "
|
|
||||||
f"session_id={owner_session}; explicit reclaim not implemented "
|
|
||||||
"(fail closed)"
|
|
||||||
)
|
|
||||||
|
|
||||||
if not (reviewer_identity or "").strip():
|
|
||||||
reasons.append("reviewer identity required for lease acquisition")
|
|
||||||
if not (session_id or "").strip():
|
|
||||||
reasons.append("session_id required for lease acquisition")
|
|
||||||
if not (worktree or "").strip():
|
|
||||||
reasons.append("worktree path required for lease acquisition")
|
|
||||||
|
|
||||||
allowed = not reasons
|
|
||||||
body = None
|
|
||||||
if allowed:
|
|
||||||
body = format_lease_body(
|
|
||||||
repo=repo,
|
|
||||||
pr_number=pr_number,
|
|
||||||
issue_number=issue_number,
|
|
||||||
reviewer_identity=reviewer_identity,
|
|
||||||
profile=profile,
|
|
||||||
session_id=session_id,
|
|
||||||
worktree=worktree,
|
|
||||||
phase="claimed",
|
|
||||||
candidate_head=candidate_head,
|
|
||||||
target_branch=target_branch,
|
|
||||||
target_branch_sha=target_branch_sha,
|
|
||||||
last_activity=now,
|
|
||||||
)
|
|
||||||
return {
|
|
||||||
"acquire_allowed": allowed,
|
|
||||||
"reasons": reasons,
|
|
||||||
"existing_lease": existing,
|
|
||||||
"lease_body": body,
|
|
||||||
"session_id": session_id,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def record_session_lease(lease: dict[str, Any]) -> dict[str, Any]:
|
|
||||||
global _SESSION_LEASE
|
|
||||||
_SESSION_LEASE = dict(lease)
|
|
||||||
return dict(_SESSION_LEASE)
|
|
||||||
|
|
||||||
|
|
||||||
def clear_session_lease() -> None:
|
|
||||||
global _SESSION_LEASE
|
|
||||||
_SESSION_LEASE = None
|
|
||||||
|
|
||||||
|
|
||||||
def get_session_lease() -> dict[str, Any] | None:
|
|
||||||
return dict(_SESSION_LEASE) if _SESSION_LEASE else None
|
|
||||||
|
|
||||||
|
|
||||||
def assess_mutation_lease_gate(
|
|
||||||
*,
|
|
||||||
pr_number: int,
|
|
||||||
comments: list[dict],
|
|
||||||
reviewer_identity: str,
|
|
||||||
session_id: str | None,
|
|
||||||
mutation: str,
|
|
||||||
live_head_sha: str | None,
|
|
||||||
pinned_head_sha: str | None,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Reviewer mutations require an owned, current PR lease."""
|
|
||||||
now = now or datetime.now(timezone.utc)
|
|
||||||
reasons: list[str] = []
|
|
||||||
session = get_session_lease()
|
|
||||||
active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
|
|
||||||
|
|
||||||
if not session:
|
|
||||||
reasons.append(
|
|
||||||
f"no in-session reviewer lease recorded; acquire via "
|
|
||||||
f"gitea_acquire_reviewer_pr_lease before {mutation}"
|
|
||||||
)
|
|
||||||
elif session.get("pr_number") != pr_number:
|
|
||||||
reasons.append(
|
|
||||||
f"session lease is for PR #{session.get('pr_number')}, not #{pr_number}"
|
|
||||||
)
|
|
||||||
elif (session.get("session_id") or "") != (session_id or session.get("session_id")):
|
|
||||||
reasons.append("session lease session_id mismatch (fail closed)")
|
|
||||||
|
|
||||||
if active:
|
|
||||||
owner = (active.get("session_id") or "").strip()
|
|
||||||
if owner and session_id and owner != session_id:
|
|
||||||
reasons.append(
|
|
||||||
f"active PR lease owned by session_id={owner}; current session "
|
|
||||||
f"cannot {mutation}"
|
|
||||||
)
|
|
||||||
pinned = _normalize_sha(pinned_head_sha)
|
|
||||||
live = _normalize_sha(live_head_sha)
|
|
||||||
lease_head = active.get("candidate_head")
|
|
||||||
if pinned and live and pinned != live:
|
|
||||||
reasons.append(
|
|
||||||
"PR head changed during lease; stop and re-validate before "
|
|
||||||
f"reviewer {mutation}"
|
|
||||||
)
|
|
||||||
if lease_head and live and lease_head != live:
|
|
||||||
reasons.append(
|
|
||||||
"live PR head differs from lease candidate_head; refresh lease "
|
|
||||||
f"before {mutation}"
|
|
||||||
)
|
|
||||||
freshness = active.get("freshness") or classify_lease_freshness(active, now=now)
|
|
||||||
if freshness in {"expired", "reclaimable"}:
|
|
||||||
reasons.append(f"reviewer lease freshness is '{freshness}' (fail closed)")
|
|
||||||
else:
|
|
||||||
reasons.append(f"no active reviewer lease found on PR #{pr_number}")
|
|
||||||
|
|
||||||
allowed = not reasons
|
|
||||||
return {
|
|
||||||
"mutation_allowed": allowed,
|
|
||||||
"block": not allowed,
|
|
||||||
"reasons": reasons,
|
|
||||||
"active_lease": active,
|
|
||||||
"session_lease": session,
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def assess_lease_inventory(
|
|
||||||
comments_by_pr: dict[int, list[dict]],
|
|
||||||
*,
|
|
||||||
now: datetime | None = None,
|
|
||||||
) -> dict[str, Any]:
|
|
||||||
"""Summarize lease states across PR comment threads."""
|
|
||||||
now = now or datetime.now(timezone.utc)
|
|
||||||
active: list[dict] = []
|
|
||||||
stale: list[dict] = []
|
|
||||||
reclaimable: list[dict] = []
|
|
||||||
for pr_number, comments in (comments_by_pr or {}).items():
|
|
||||||
lease = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
|
|
||||||
if not lease:
|
|
||||||
continue
|
|
||||||
freshness = lease.get("freshness") or classify_lease_freshness(lease, now=now)
|
|
||||||
entry = {"pr_number": pr_number, "session_id": lease.get("session_id"), "freshness": freshness}
|
|
||||||
if freshness == "stale_warning":
|
|
||||||
stale.append(entry)
|
|
||||||
elif freshness == "reclaimable":
|
|
||||||
reclaimable.append(entry)
|
|
||||||
else:
|
|
||||||
active.append(entry)
|
|
||||||
return {
|
|
||||||
"active_review_leases": active,
|
|
||||||
"stale_review_leases": stale,
|
|
||||||
"reclaimable_review_leases": reclaimable,
|
|
||||||
}
|
|
||||||
+5
-11
@@ -38,21 +38,13 @@ fi
|
|||||||
branch="$1"
|
branch="$1"
|
||||||
start_ref="${2:-prgs/master}"
|
start_ref="${2:-prgs/master}"
|
||||||
|
|
||||||
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
||||||
repo_root="$(cd "$script_dir/.." && pwd)"
|
|
||||||
|
|
||||||
# Enforce issue-linked, traceable branch names (issue → branch → worktree → PR).
|
# Enforce issue-linked, traceable branch names (issue → branch → worktree → PR).
|
||||||
if [[ "$allow_unlinked" -eq 0 ]]; then
|
if [[ "$allow_unlinked" -eq 0 ]]; then
|
||||||
locked_branch=$(python3 -c "
|
if [[ ! -f "/tmp/gitea_issue_lock.json" ]]; then
|
||||||
import sys
|
echo "Error: Issue lock file '/tmp/gitea_issue_lock.json' is missing. You must lock exactly one issue before branch creation (fail closed)." >&2
|
||||||
sys.path.insert(0, '$repo_root')
|
|
||||||
import issue_lock_store
|
|
||||||
print(issue_lock_store.resolve_locked_branch_for_session('$branch'))
|
|
||||||
")
|
|
||||||
if [[ -z "$locked_branch" ]]; then
|
|
||||||
echo "Error: No session issue lock is bound. Call gitea_lock_issue before branch creation (fail closed)." >&2
|
|
||||||
exit 2
|
exit 2
|
||||||
fi
|
fi
|
||||||
|
locked_branch=$(python3 -c "import json; print(json.load(open('/tmp/gitea_issue_lock.json')).get('branch_name', ''))")
|
||||||
if [[ "$branch" != "$locked_branch" ]]; then
|
if [[ "$branch" != "$locked_branch" ]]; then
|
||||||
echo "Error: Requested branch '$branch' does not match locked branch '$locked_branch' (fail closed)." >&2
|
echo "Error: Requested branch '$branch' does not match locked branch '$locked_branch' (fail closed)." >&2
|
||||||
exit 2
|
exit 2
|
||||||
@@ -76,6 +68,8 @@ EOF
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
repo_root="$(cd "$script_dir/.." && pwd)"
|
||||||
worktree_name="${branch//\//-}"
|
worktree_name="${branch//\//-}"
|
||||||
worktree_path="$repo_root/branches/$worktree_name"
|
worktree_path="$repo_root/branches/$worktree_name"
|
||||||
|
|
||||||
|
|||||||
@@ -732,26 +732,6 @@ The final report must identify:
|
|||||||
* whether same-PR merge continuation was allowed
|
* whether same-PR merge continuation was allowed
|
||||||
* whether the run stopped as required
|
* whether the run stopped as required
|
||||||
|
|
||||||
## 26B. Per-PR reviewer lease (#407)
|
|
||||||
|
|
||||||
Parallel reviewer sessions are allowed only when each session holds a distinct,
|
|
||||||
live PR lease.
|
|
||||||
|
|
||||||
Before validation or review mutation on a selected PR:
|
|
||||||
|
|
||||||
1. Call `gitea_acquire_reviewer_pr_lease` with worktree path, candidate head SHA,
|
|
||||||
and target branch SHA.
|
|
||||||
2. Post heartbeats via `gitea_heartbeat_reviewer_pr_lease` before validation,
|
|
||||||
after validation, before review mutation, and before merge.
|
|
||||||
3. Do not approve, request changes, or merge unless the in-session lease
|
|
||||||
matches the selected PR.
|
|
||||||
|
|
||||||
If PR head or target branch advances during the lease, stop and refresh
|
|
||||||
inventory before continuing.
|
|
||||||
|
|
||||||
Final reports must include lease session id, acquisition proof, heartbeat
|
|
||||||
status, and release/blocked status.
|
|
||||||
|
|
||||||
## 27. Merge rules
|
## 27. Merge rules
|
||||||
|
|
||||||
Before merge, rerun fresh live checks:
|
Before merge, rerun fresh live checks:
|
||||||
|
|||||||
@@ -297,36 +297,6 @@ Report:
|
|||||||
|
|
||||||
Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven.
|
Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven.
|
||||||
|
|
||||||
### 10A. Duplicate-work gate phases (#400)
|
|
||||||
|
|
||||||
Before any file edits, prove duplicate-work clearance with
|
|
||||||
`gitea_assess_work_issue_duplicate` or `gitea_lock_issue` (which runs the same
|
|
||||||
gate). The gate checks live:
|
|
||||||
|
|
||||||
* open PRs linked to the issue (head branch or Closes/Fixes reference),
|
|
||||||
* remote branches matching `issue-<number>`,
|
|
||||||
* active claim leases from structured heartbeats.
|
|
||||||
|
|
||||||
Re-check immediately before:
|
|
||||||
|
|
||||||
* `gitea_commit_files` (commit),
|
|
||||||
* branch push,
|
|
||||||
* `gitea_create_pr` (PR creation).
|
|
||||||
|
|
||||||
If a concurrent open PR appears after work begins:
|
|
||||||
|
|
||||||
* before commit/push → stop and preserve local work without pushing,
|
|
||||||
* after commit but before push → stop without pushing,
|
|
||||||
* after push but before PR creation → stop and produce a reconciliation
|
|
||||||
handoff instead of opening a PR.
|
|
||||||
|
|
||||||
Final reports must state exactly one duplicate-work outcome:
|
|
||||||
|
|
||||||
* `duplicate PR prevented`
|
|
||||||
* `duplicate branch prevented`
|
|
||||||
* `duplicate commit prevented`
|
|
||||||
* `duplicate work not prevented`
|
|
||||||
|
|
||||||
## 11. Claim or lock the issue before implementation
|
## 11. Claim or lock the issue before implementation
|
||||||
|
|
||||||
Claim/lock the issue before implementation if the project provides a claim/lock mechanism.
|
Claim/lock the issue before implementation if the project provides a claim/lock mechanism.
|
||||||
@@ -747,12 +717,6 @@ Use only precise categories:
|
|||||||
* External-state mutations:
|
* External-state mutations:
|
||||||
* Read-only diagnostics:
|
* Read-only diagnostics:
|
||||||
|
|
||||||
Issue-lock file (`/tmp/gitea_issue_lock.json`) read/write/delete is always an
|
|
||||||
external-state mutation. Never claim `External-state mutations: none` after
|
|
||||||
seeding, restoring, or removing that file. Manual lock seeding is not a normal
|
|
||||||
recovery path (#447); use `gitea_lock_issue` or the #442 adoption recovery path
|
|
||||||
instead. Link broader redesign: #438.
|
|
||||||
|
|
||||||
`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics.
|
`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics.
|
||||||
|
|
||||||
If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`.
|
If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`.
|
||||||
|
|||||||
@@ -74,24 +74,18 @@ ISSUE_WRITE_ENV = {
|
|||||||
|
|
||||||
class TestIssueLockArtifactWarning(unittest.TestCase):
|
class TestIssueLockArtifactWarning(unittest.TestCase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
self._lock_dir = tempfile.TemporaryDirectory()
|
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||||
env = {**ISSUE_WRITE_ENV, "GITEA_ISSUE_LOCK_DIR": self._lock_dir.name}
|
|
||||||
self._env_patcher = patch.dict(os.environ, env, clear=True)
|
|
||||||
self._env_patcher.start()
|
self._env_patcher.start()
|
||||||
|
|
||||||
def tearDown(self):
|
def tearDown(self):
|
||||||
self._env_patcher.stop()
|
self._env_patcher.stop()
|
||||||
self._lock_dir.cleanup()
|
|
||||||
|
|
||||||
@patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
@patch("mcp_server.api_get_all", return_value=[])
|
@patch("mcp_server.api_get_all", return_value=[])
|
||||||
@patch("mcp_server._auth", return_value="token x")
|
@patch("mcp_server._auth", return_value="token x")
|
||||||
@patch("mcp_server._resolve", return_value=("h", "o", "r"))
|
@patch("mcp_server._resolve", return_value=("h", "o", "r"))
|
||||||
|
@patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp())
|
||||||
@patch("issue_lock_worktree.read_worktree_git_state")
|
@patch("issue_lock_worktree.read_worktree_git_state")
|
||||||
def test_lock_success_includes_artifact_warning(self, mock_state, *_mocks):
|
def test_lock_success_includes_artifact_warning(self, mock_state, _lock_file, *_mocks):
|
||||||
mock_state.return_value = {
|
mock_state.return_value = {
|
||||||
"current_branch": "master",
|
"current_branch": "master",
|
||||||
"porcelain_status": "?? _emit_payload.py\n",
|
"porcelain_status": "?? _emit_payload.py\n",
|
||||||
|
|||||||
+2
-28
@@ -286,21 +286,6 @@ class TestSimpleToolAudit(_AuditWiringBase):
|
|||||||
|
|
||||||
class TestGatedToolAudit(_AuditWiringBase):
|
class TestGatedToolAudit(_AuditWiringBase):
|
||||||
|
|
||||||
def setUp(self):
|
|
||||||
super().setUp()
|
|
||||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
self._lease_patch = _install_owned_reviewer_lease(8)
|
|
||||||
self._lease_patch.start()
|
|
||||||
self._auth_identity_patch = patch(
|
|
||||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
|
||||||
)
|
|
||||||
self._auth_identity_patch.start()
|
|
||||||
self.addCleanup(self._auth_identity_patch.stop)
|
|
||||||
self.addCleanup(self._lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||||
return {"user": {"login": author}, "state": state,
|
return {"user": {"login": author}, "state": state,
|
||||||
"head": {"sha": sha}, "mergeable": mergeable}
|
"head": {"sha": sha}, "mergeable": mergeable}
|
||||||
@@ -359,22 +344,11 @@ class TestGatedToolAudit(_AuditWiringBase):
|
|||||||
GITEA_ALLOWED_OPERATIONS="read,review,approve")
|
GITEA_ALLOWED_OPERATIONS="read,review,approve")
|
||||||
with patch.dict(os.environ, env, clear=True):
|
with patch.dict(os.environ, env, clear=True):
|
||||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
init_review_decision_lock("prgs", "review_pr")
|
init_review_decision_lock("prgs", "review_pr")
|
||||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||||
lease_patch = _install_owned_reviewer_lease(8)
|
r = gitea_submit_pr_review(pr_number=8, action="approve",
|
||||||
lease_patch.start()
|
|
||||||
try:
|
|
||||||
r = gitea_submit_pr_review(
|
|
||||||
pr_number=8, action="approve",
|
|
||||||
body="LGTM", remote="prgs",
|
body="LGTM", remote="prgs",
|
||||||
final_review_decision_ready=True,
|
final_review_decision_ready=True)
|
||||||
)
|
|
||||||
finally:
|
|
||||||
lease_patch.stop()
|
|
||||||
reviewer_pr_lease.clear_session_lease()
|
|
||||||
self.assertTrue(r["performed"])
|
self.assertTrue(r["performed"])
|
||||||
recs = self._records()
|
recs = self._records()
|
||||||
self.assertEqual(len(recs), 1)
|
self.assertEqual(len(recs), 1)
|
||||||
|
|||||||
@@ -76,14 +76,7 @@ class CommitFilesCapabilityBase(unittest.TestCase):
|
|||||||
with open(self.config_path, "w", encoding="utf-8") as fh:
|
with open(self.config_path, "w", encoding="utf-8") as fh:
|
||||||
fh.write(json.dumps(CONFIG))
|
fh.write(json.dumps(CONFIG))
|
||||||
|
|
||||||
self._dup_fetcher_patcher = patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
self._dup_fetcher_patcher.start()
|
|
||||||
|
|
||||||
def tearDown(self):
|
def tearDown(self):
|
||||||
self._dup_fetcher_patcher.stop()
|
|
||||||
self._remotes.stop()
|
self._remotes.stop()
|
||||||
mcp_server._IDENTITY_CACHE.clear()
|
mcp_server._IDENTITY_CACHE.clear()
|
||||||
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
|
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
|
||||||
|
|||||||
@@ -66,19 +66,7 @@ class TestCommitPayloads(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name)
|
self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name)
|
||||||
|
|
||||||
import issue_lock_store
|
self.lock_file_path = "/tmp/gitea_issue_lock.json"
|
||||||
import issue_lock_provenance
|
|
||||||
|
|
||||||
self._lock_dir = tempfile.TemporaryDirectory()
|
|
||||||
os.environ["GITEA_ISSUE_LOCK_DIR"] = self._lock_dir.name
|
|
||||||
|
|
||||||
work_lease = {
|
|
||||||
"operation_type": "author_issue_work",
|
|
||||||
"issue_number": 263,
|
|
||||||
"branch": "feat/issue-263-native-commit-payloads",
|
|
||||||
"claimant": {"username": "test-user", "profile": "test-author"},
|
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
|
||||||
}
|
|
||||||
self.lock_data = {
|
self.lock_data = {
|
||||||
"issue_number": 263,
|
"issue_number": 263,
|
||||||
"branch_name": "feat/issue-263-native-commit-payloads",
|
"branch_name": "feat/issue-263-native-commit-payloads",
|
||||||
@@ -86,13 +74,9 @@ class TestCommitPayloads(unittest.TestCase):
|
|||||||
"org": "Example-Org",
|
"org": "Example-Org",
|
||||||
"repo": "Example-Repo",
|
"repo": "Example-Repo",
|
||||||
"worktree_path": self.locked_worktree_path,
|
"worktree_path": self.locked_worktree_path,
|
||||||
"work_lease": work_lease,
|
|
||||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
|
||||||
tool="gitea_lock_issue",
|
|
||||||
claimant=work_lease.get("claimant"),
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
self.lock_file_path = issue_lock_store.bind_session_lock(self.lock_data)
|
with open(self.lock_file_path, "w", encoding="utf-8") as fh:
|
||||||
|
fh.write(json.dumps(self.lock_data))
|
||||||
|
|
||||||
# Reset preflight status to bypass/pass verification in tests
|
# Reset preflight status to bypass/pass verification in tests
|
||||||
self.orig_whoami_called = mcp_server._preflight_whoami_called
|
self.orig_whoami_called = mcp_server._preflight_whoami_called
|
||||||
@@ -100,14 +84,7 @@ class TestCommitPayloads(unittest.TestCase):
|
|||||||
mcp_server._preflight_whoami_called = True
|
mcp_server._preflight_whoami_called = True
|
||||||
mcp_server._preflight_capability_called = True
|
mcp_server._preflight_capability_called = True
|
||||||
|
|
||||||
self._dup_fetcher_patcher = patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
self._dup_fetcher_patcher.start()
|
|
||||||
|
|
||||||
def tearDown(self):
|
def tearDown(self):
|
||||||
self._dup_fetcher_patcher.stop()
|
|
||||||
self._remotes.stop()
|
self._remotes.stop()
|
||||||
mcp_server._IDENTITY_CACHE.clear()
|
mcp_server._IDENTITY_CACHE.clear()
|
||||||
|
|
||||||
@@ -116,7 +93,8 @@ class TestCommitPayloads(unittest.TestCase):
|
|||||||
|
|
||||||
self._dir.cleanup()
|
self._dir.cleanup()
|
||||||
self.locked_worktree_dir.cleanup()
|
self.locked_worktree_dir.cleanup()
|
||||||
self._lock_dir.cleanup()
|
if os.path.exists(self.lock_file_path):
|
||||||
|
os.remove(self.lock_file_path)
|
||||||
|
|
||||||
def _env(self, profile: str) -> dict:
|
def _env(self, profile: str) -> dict:
|
||||||
return {
|
return {
|
||||||
@@ -125,7 +103,6 @@ class TestCommitPayloads(unittest.TestCase):
|
|||||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||||
"GITEA_TEST_PORCELAIN": "",
|
"GITEA_TEST_PORCELAIN": "",
|
||||||
"GITEA_AUTHOR_WORKTREE": self.locked_worktree_path,
|
"GITEA_AUTHOR_WORKTREE": self.locked_worktree_path,
|
||||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
|
|||||||
@@ -106,28 +106,16 @@ class TestCreateIssueWorkspaceGuard(unittest.TestCase):
|
|||||||
@patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"})
|
@patch("gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", return_value={"current_branch": "feat/issue-1"})
|
||||||
@patch("os.path.exists", return_value=True)
|
@patch("os.path.exists", return_value=True)
|
||||||
@patch("os.path.isdir", return_value=True)
|
@patch("os.path.isdir", return_value=True)
|
||||||
@patch("author_mutation_worktree.subprocess.run")
|
|
||||||
@patch("subprocess.run")
|
@patch("subprocess.run")
|
||||||
def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_amw_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth):
|
def test_create_issue_wrong_repo_fails_closed(self, mock_run, mock_isdir, mock_exists, _git, _get_all, mock_api, _role, _ns, _prof, _auth):
|
||||||
|
# Mock subprocess.run for git --git-common-dir to return a different path
|
||||||
|
mock_res = MagicMock()
|
||||||
|
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
|
||||||
|
mock_run.return_value = mock_res
|
||||||
|
|
||||||
wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1")
|
wrong_repo_path = os.path.join(CONTROL_CHECKOUT_ROOT, "branches", "feat-issue-1")
|
||||||
|
|
||||||
def _subprocess_side_effect(cmd, *args, **kwargs):
|
|
||||||
mock_res = MagicMock(returncode=0)
|
|
||||||
if "--git-common-dir" in cmd:
|
|
||||||
cwd = cmd[cmd.index("-C") + 1] if "-C" in cmd else ""
|
|
||||||
if cwd == wrong_repo_path:
|
|
||||||
mock_res.stdout = "/Users/jasonwalker/Development/some-other-repo/.git\n"
|
|
||||||
else:
|
|
||||||
mock_res.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n"
|
|
||||||
else:
|
|
||||||
mock_res.stdout = ""
|
|
||||||
return mock_res
|
|
||||||
|
|
||||||
mock_run.side_effect = _subprocess_side_effect
|
|
||||||
mock_amw_run.side_effect = _subprocess_side_effect
|
|
||||||
|
|
||||||
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 self.assertRaises(RuntimeError) as ctx:
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
srv.gitea_create_issue(
|
srv.gitea_create_issue(
|
||||||
title="Test issue", body="body", worktree_path=wrong_repo_path
|
title="Test issue", body="body", worktree_path=wrong_repo_path
|
||||||
|
|||||||
@@ -1,138 +0,0 @@
|
|||||||
"""Unit tests for own-branch lock adoption decision (#442 / #443)."""
|
|
||||||
import sys
|
|
||||||
import unittest
|
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
from issue_lock_adoption import ( # noqa: E402
|
|
||||||
ADOPT,
|
|
||||||
BLOCK_COMPETING,
|
|
||||||
NO_MATCH,
|
|
||||||
assess_own_branch_adoption,
|
|
||||||
build_adoption_proof,
|
|
||||||
)
|
|
||||||
|
|
||||||
REQ = "feat/issue-420-server-code-parity"
|
|
||||||
|
|
||||||
|
|
||||||
class TestAssessOwnBranchAdoption(unittest.TestCase):
|
|
||||||
def test_exact_own_branch_is_adopted(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420,
|
|
||||||
requested_branch=REQ,
|
|
||||||
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], ADOPT)
|
|
||||||
self.assertTrue(result["adopt"])
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
self.assertEqual(result["matched_branch"], REQ)
|
|
||||||
self.assertEqual(result["matched_head_sha"], "934688a")
|
|
||||||
|
|
||||||
def test_exact_own_branch_adopted_when_sha_missing(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420, requested_branch=REQ, existing_branches=[REQ]
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], ADOPT)
|
|
||||||
self.assertIsNone(result["matched_head_sha"])
|
|
||||||
|
|
||||||
def test_different_branch_same_issue_blocks(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420,
|
|
||||||
requested_branch=REQ,
|
|
||||||
existing_branches=[{"name": "feat/issue-420-other-work"}],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], BLOCK_COMPETING)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertFalse(result["adopt"])
|
|
||||||
self.assertIn("feat/issue-420-other-work", result["competing_branches"])
|
|
||||||
self.assertIn("fail closed", result["reason"])
|
|
||||||
|
|
||||||
def test_own_branch_plus_competing_branch_blocks(self):
|
|
||||||
# Ambiguous ownership: fail closed even though the exact branch exists.
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420,
|
|
||||||
requested_branch=REQ,
|
|
||||||
existing_branches=[{"name": REQ}, {"name": "feat/issue-420-rogue"}],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], BLOCK_COMPETING)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertEqual(result["competing_branches"], ["feat/issue-420-rogue"])
|
|
||||||
|
|
||||||
def test_no_matching_branch_is_normal_path(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420,
|
|
||||||
requested_branch=REQ,
|
|
||||||
existing_branches=[{"name": "feat/issue-999-unrelated"}],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], NO_MATCH)
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
self.assertFalse(result["adopt"])
|
|
||||||
|
|
||||||
def test_empty_branch_list_is_normal_path(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=420, requested_branch=REQ, existing_branches=[]
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], NO_MATCH)
|
|
||||||
|
|
||||||
def test_higher_issue_number_branch_does_not_block_lower_issue_adoption(self):
|
|
||||||
# issue-420 must not be treated as competing work for issue #42.
|
|
||||||
own_branch = "feat/issue-42-widget"
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=42,
|
|
||||||
requested_branch=own_branch,
|
|
||||||
existing_branches=[
|
|
||||||
{"name": own_branch, "commit_sha": "abc1234"},
|
|
||||||
{"name": "feat/issue-420-server-code-parity"},
|
|
||||||
],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], ADOPT)
|
|
||||||
self.assertTrue(result["adopt"])
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
self.assertEqual(result["matched_branch"], own_branch)
|
|
||||||
|
|
||||||
def test_unrelated_higher_number_branch_is_ignored_without_own_branch(self):
|
|
||||||
result = assess_own_branch_adoption(
|
|
||||||
issue_number=42,
|
|
||||||
requested_branch="feat/issue-42-thing",
|
|
||||||
existing_branches=[{"name": "feat/issue-420-server-code-parity"}],
|
|
||||||
)
|
|
||||||
self.assertEqual(result["outcome"], NO_MATCH)
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
self.assertFalse(result["adopt"])
|
|
||||||
|
|
||||||
|
|
||||||
class TestBuildAdoptionProof(unittest.TestCase):
|
|
||||||
def test_proof_has_all_required_fields(self):
|
|
||||||
assessment = assess_own_branch_adoption(
|
|
||||||
issue_number=420,
|
|
||||||
requested_branch=REQ,
|
|
||||||
existing_branches=[{"name": REQ, "commit_sha": "934688a"}],
|
|
||||||
)
|
|
||||||
proof = build_adoption_proof(
|
|
||||||
issue_number=420,
|
|
||||||
branch_name=REQ,
|
|
||||||
assessment=assessment,
|
|
||||||
open_pr_checked=True,
|
|
||||||
competing_lock_checked=True,
|
|
||||||
lock_file_path="/tmp/example-lock.json",
|
|
||||||
lock_file_status="written",
|
|
||||||
)
|
|
||||||
for key in (
|
|
||||||
"issue_number",
|
|
||||||
"branch_name",
|
|
||||||
"branch_head_commit",
|
|
||||||
"adoption_reason",
|
|
||||||
"no_existing_pr_proof",
|
|
||||||
"no_competing_live_lock_proof",
|
|
||||||
"lock_file_path",
|
|
||||||
"lock_file_status",
|
|
||||||
):
|
|
||||||
self.assertIn(key, proof)
|
|
||||||
self.assertEqual(proof["branch_head_commit"], "934688a")
|
|
||||||
self.assertTrue(proof["no_existing_pr_proof"])
|
|
||||||
self.assertTrue(proof["no_competing_live_lock_proof"])
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
@@ -1,130 +0,0 @@
|
|||||||
"""Tests for issue-lock provenance and external-state disclosure (#447)."""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import sys
|
|
||||||
import unittest
|
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import issue_lock_provenance as ilp # noqa: E402
|
|
||||||
from final_report_validator import assess_final_report_validator # noqa: E402
|
|
||||||
|
|
||||||
|
|
||||||
def _sanctioned_lock(**overrides):
|
|
||||||
work_lease = {
|
|
||||||
"operation_type": "author_issue_work",
|
|
||||||
"issue_number": 447,
|
|
||||||
"branch": "feat/issue-447-lock-provenance",
|
|
||||||
"claimant": {"username": "jcwalker3", "profile": "prgs-author"},
|
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
|
||||||
}
|
|
||||||
data = {
|
|
||||||
"issue_number": 447,
|
|
||||||
"branch_name": "feat/issue-447-lock-provenance",
|
|
||||||
"work_lease": work_lease,
|
|
||||||
"lock_provenance": ilp.build_sanctioned_lock_provenance(
|
|
||||||
tool="gitea_lock_issue",
|
|
||||||
claimant=work_lease["claimant"],
|
|
||||||
),
|
|
||||||
}
|
|
||||||
data.update(overrides)
|
|
||||||
return data
|
|
||||||
|
|
||||||
|
|
||||||
class TestLockProvenanceForCreatePr(unittest.TestCase):
|
|
||||||
def test_sanctioned_lock_passes(self):
|
|
||||||
result = ilp.assess_lock_file_for_create_pr(_sanctioned_lock())
|
|
||||||
self.assertTrue(result["proven"])
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
|
|
||||||
def test_manual_seed_without_provenance_blocked(self):
|
|
||||||
result = ilp.assess_lock_file_for_create_pr(
|
|
||||||
{"issue_number": 420, "branch_name": "feat/x", "work_lease": {}}
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertIn("lock_provenance", result["reasons"][0])
|
|
||||||
|
|
||||||
def test_operator_override_requires_reason(self):
|
|
||||||
result = ilp.assess_lock_file_for_create_pr(
|
|
||||||
_sanctioned_lock(
|
|
||||||
lock_provenance=ilp.build_sanctioned_lock_provenance(
|
|
||||||
tool="operator_override",
|
|
||||||
source=ilp.SOURCE_OPERATOR_OVERRIDE,
|
|
||||||
)
|
|
||||||
)
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
|
|
||||||
class TestExternalStateReportRules(unittest.TestCase):
|
|
||||||
def test_seed_with_external_none_blocked(self):
|
|
||||||
report = (
|
|
||||||
"Restored /tmp/gitea_issue_lock.json to unblock PR creation.\n"
|
|
||||||
"- External-state mutations: none\n"
|
|
||||||
)
|
|
||||||
result = ilp.assess_issue_lock_external_state_report(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_seed_with_disclosure_passes(self):
|
|
||||||
report = (
|
|
||||||
"Restored /tmp/gitea_issue_lock.json after MCP restart.\n"
|
|
||||||
"- External-state mutations: wrote /tmp/gitea_issue_lock.json\n"
|
|
||||||
)
|
|
||||||
result = ilp.assess_issue_lock_external_state_report(report)
|
|
||||||
self.assertTrue(result["proven"])
|
|
||||||
|
|
||||||
def test_remove_claimed_as_cleanup_only_blocked(self):
|
|
||||||
report = (
|
|
||||||
"rm /tmp/gitea_issue_lock.json after PR creation.\n"
|
|
||||||
"- Cleanup mutations: lock removed\n"
|
|
||||||
"- External-state mutations: none\n"
|
|
||||||
)
|
|
||||||
result = ilp.assess_issue_lock_external_state_report(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_manual_lock_pr_without_override_blocked(self):
|
|
||||||
report = (
|
|
||||||
"Programmatically seeded gitea_issue_lock.json then gitea_create_pr.\n"
|
|
||||||
"PR #444 created.\n"
|
|
||||||
)
|
|
||||||
result = ilp.assess_manual_lock_pr_without_override(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_author_reviewer_same_run_blocked(self):
|
|
||||||
report = (
|
|
||||||
"gitea_create_pr opened PR #444.\n"
|
|
||||||
"Submitted approve review on PR #444.\n"
|
|
||||||
)
|
|
||||||
result = ilp.assess_author_reviewer_same_run_report(report)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
|
|
||||||
class TestFinalReportValidatorIntegration(unittest.TestCase):
|
|
||||||
def test_work_issue_blocks_hidden_lock_mutation(self):
|
|
||||||
report = (
|
|
||||||
"## Controller Handoff\n"
|
|
||||||
"- Task: work issue #420\n"
|
|
||||||
"- External-state mutations: none\n"
|
|
||||||
"Restored /tmp/gitea_issue_lock.json before PR creation.\n"
|
|
||||||
)
|
|
||||||
result = assess_final_report_validator(report, "work_issue")
|
|
||||||
rule_ids = {f["rule_id"] for f in result["findings"]}
|
|
||||||
self.assertIn("shared.issue_lock_external_state", rule_ids)
|
|
||||||
self.assertTrue(result["blocked"])
|
|
||||||
|
|
||||||
def test_review_pr_blocks_create_and_approve(self):
|
|
||||||
report = (
|
|
||||||
"## Controller Handoff\n"
|
|
||||||
"- Task: review PR #444\n"
|
|
||||||
"- Review decision: approve\n"
|
|
||||||
"Created PR #444 via gitea_create_pr earlier in this run.\n"
|
|
||||||
)
|
|
||||||
result = assess_final_report_validator(report, "review_pr")
|
|
||||||
rule_ids = {f["rule_id"] for f in result["findings"]}
|
|
||||||
self.assertIn("shared.author_reviewer_same_run", rule_ids)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
@@ -1,181 +0,0 @@
|
|||||||
"""Unit tests for keyed issue-lock storage (#443)."""
|
|
||||||
import json
|
|
||||||
import os
|
|
||||||
import sys
|
|
||||||
import tempfile
|
|
||||||
import unittest
|
|
||||||
from datetime import datetime, timedelta, timezone
|
|
||||||
from pathlib import Path
|
|
||||||
from unittest import mock
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import issue_lock_store as ils # noqa: E402
|
|
||||||
|
|
||||||
|
|
||||||
def _lease(expires_at: str) -> dict:
|
|
||||||
return {
|
|
||||||
"operation_type": ils.AUTHOR_ISSUE_WORK_LEASE,
|
|
||||||
"expires_at": expires_at,
|
|
||||||
"created_at": "2026-01-01T00:00:00Z",
|
|
||||||
"last_heartbeat_at": "2026-01-01T00:00:00Z",
|
|
||||||
}
|
|
||||||
|
|
||||||
|
|
||||||
def _lock_record(**overrides) -> dict:
|
|
||||||
record = {
|
|
||||||
"issue_number": 420,
|
|
||||||
"branch_name": "feat/issue-420-server-code-parity",
|
|
||||||
"remote": "prgs",
|
|
||||||
"org": "Scaled-Tech-Consulting",
|
|
||||||
"repo": "Gitea-Tools",
|
|
||||||
"worktree_path": "/tmp/wt-420",
|
|
||||||
"work_lease": _lease("2999-01-01T00:00:00Z"),
|
|
||||||
}
|
|
||||||
record.update(overrides)
|
|
||||||
return record
|
|
||||||
|
|
||||||
|
|
||||||
class TestIssueLockStore(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
self._dir = tempfile.TemporaryDirectory()
|
|
||||||
self.lock_dir = self._dir.name
|
|
||||||
self._env = mock.patch.dict(os.environ, {"GITEA_ISSUE_LOCK_DIR": self.lock_dir})
|
|
||||||
self._env.start()
|
|
||||||
|
|
||||||
def tearDown(self):
|
|
||||||
self._env.stop()
|
|
||||||
self._dir.cleanup()
|
|
||||||
|
|
||||||
def test_concurrent_repo_locks_do_not_overwrite(self):
|
|
||||||
lock_a = _lock_record(
|
|
||||||
issue_number=108,
|
|
||||||
branch_name="feat/issue-108-root-menu",
|
|
||||||
repo="mcp-control-plane",
|
|
||||||
worktree_path="/tmp/wt-108",
|
|
||||||
)
|
|
||||||
lock_b = _lock_record(
|
|
||||||
issue_number=420,
|
|
||||||
branch_name="feat/issue-420-server-code-parity",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
worktree_path="/tmp/wt-420",
|
|
||||||
)
|
|
||||||
path_a = ils.bind_session_lock(lock_a)
|
|
||||||
with mock.patch("os.getpid", return_value=9999):
|
|
||||||
path_b = ils.bind_session_lock(lock_b)
|
|
||||||
|
|
||||||
self.assertNotEqual(path_a, path_b)
|
|
||||||
self.assertTrue(os.path.exists(path_a))
|
|
||||||
self.assertTrue(os.path.exists(path_b))
|
|
||||||
stored_a = ils.read_lock_file(path_a)
|
|
||||||
stored_b = ils.read_lock_file(path_b)
|
|
||||||
self.assertEqual(stored_a["issue_number"], 108)
|
|
||||||
self.assertEqual(stored_b["issue_number"], 420)
|
|
||||||
|
|
||||||
def test_concurrent_issue_locks_same_repo_do_not_overwrite(self):
|
|
||||||
lock_a = _lock_record(issue_number=427, branch_name="feat/issue-427-a")
|
|
||||||
lock_b = _lock_record(issue_number=428, branch_name="feat/issue-428-b")
|
|
||||||
path_a = ils.bind_session_lock(lock_a)
|
|
||||||
with mock.patch("os.getpid", return_value=4242):
|
|
||||||
path_b = ils.bind_session_lock(lock_b)
|
|
||||||
|
|
||||||
self.assertNotEqual(path_a, path_b)
|
|
||||||
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 427)
|
|
||||||
self.assertEqual(ils.read_lock_file(path_b)["issue_number"], 428)
|
|
||||||
|
|
||||||
def test_foreign_live_lease_blocks_overwrite(self):
|
|
||||||
existing = _lock_record(
|
|
||||||
branch_name="feat/issue-420-other",
|
|
||||||
worktree_path="/tmp/other",
|
|
||||||
work_lease=_lease("2999-01-01T00:00:00Z"),
|
|
||||||
)
|
|
||||||
path = ils.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
issue_number=420,
|
|
||||||
)
|
|
||||||
ils.save_lock_file(path, existing)
|
|
||||||
|
|
||||||
incoming = _lock_record(worktree_path="/tmp/mine")
|
|
||||||
block = ils.assess_foreign_lock_overwrite(existing, incoming)
|
|
||||||
self.assertIn("live foreign issue lock", block or "")
|
|
||||||
|
|
||||||
def test_expired_lease_allows_takeover_with_conflict_check(self):
|
|
||||||
existing = _lock_record(
|
|
||||||
branch_name="feat/issue-420-other",
|
|
||||||
worktree_path="/tmp/other",
|
|
||||||
work_lease=_lease("2000-01-01T00:00:00Z"),
|
|
||||||
)
|
|
||||||
incoming = _lock_record(worktree_path="/tmp/mine")
|
|
||||||
self.assertIsNone(ils.assess_foreign_lock_overwrite(existing, incoming))
|
|
||||||
block = ils.assess_same_issue_lease_conflict(
|
|
||||||
existing,
|
|
||||||
issue_number=420,
|
|
||||||
branch_name="feat/issue-420-server-code-parity",
|
|
||||||
worktree_path="/tmp/mine",
|
|
||||||
)
|
|
||||||
self.assertIn("Recovery review is required", block or "")
|
|
||||||
|
|
||||||
def test_same_owner_lease_conflict_allows_refresh(self):
|
|
||||||
worktree = "/tmp/wt-420"
|
|
||||||
existing = _lock_record(worktree_path=worktree)
|
|
||||||
block = ils.assess_same_issue_lease_conflict(
|
|
||||||
existing,
|
|
||||||
issue_number=420,
|
|
||||||
branch_name="feat/issue-420-server-code-parity",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIsNone(block)
|
|
||||||
|
|
||||||
def test_find_lock_for_branch_after_restart(self):
|
|
||||||
record = _lock_record()
|
|
||||||
path = ils.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
issue_number=420,
|
|
||||||
)
|
|
||||||
ils.save_lock_file(path, record)
|
|
||||||
|
|
||||||
with mock.patch("os.getpid", return_value=5555):
|
|
||||||
self.assertIsNone(ils.read_session_issue_lock())
|
|
||||||
|
|
||||||
found = ils.find_lock_for_branch(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
branch_name="feat/issue-420-server-code-parity",
|
|
||||||
)
|
|
||||||
self.assertEqual(found["issue_number"], 420)
|
|
||||||
|
|
||||||
def test_has_active_issue_lock_scans_keyed_store(self):
|
|
||||||
ils.bind_session_lock(_lock_record())
|
|
||||||
self.assertTrue(
|
|
||||||
ils.has_active_issue_lock("feat/issue-420-server-code-parity")
|
|
||||||
)
|
|
||||||
self.assertFalse(ils.has_active_issue_lock("feat/issue-999-other"))
|
|
||||||
|
|
||||||
def test_atomic_write_preserves_unrelated_lock(self):
|
|
||||||
path_a = ils.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
issue_number=108,
|
|
||||||
)
|
|
||||||
ils.save_lock_file(path_a, _lock_record(issue_number=108, repo="mcp-control-plane"))
|
|
||||||
path_b = ils.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
issue_number=420,
|
|
||||||
)
|
|
||||||
ils.save_lock_file(path_b, _lock_record())
|
|
||||||
|
|
||||||
self.assertTrue(os.path.exists(path_a))
|
|
||||||
self.assertTrue(os.path.exists(path_b))
|
|
||||||
self.assertEqual(ils.read_lock_file(path_a)["issue_number"], 108)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
@@ -1,272 +0,0 @@
|
|||||||
"""Tests for early duplicate-work detection (#400)."""
|
|
||||||
import os
|
|
||||||
import sys
|
|
||||||
import tempfile
|
|
||||||
import unittest
|
|
||||||
from pathlib import Path
|
|
||||||
from unittest.mock import patch
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import issue_lock_provenance
|
|
||||||
import issue_lock_store
|
|
||||||
import issue_work_duplicate_gate as dup_gate
|
|
||||||
import mcp_server
|
|
||||||
from issue_work_duplicate_gate import (
|
|
||||||
OUTCOME_DUPLICATE_BRANCH_PREVENTED,
|
|
||||||
OUTCOME_DUPLICATE_COMMIT_PREVENTED,
|
|
||||||
OUTCOME_DUPLICATE_PR_PREVENTED,
|
|
||||||
OUTCOME_DUPLICATE_WORK_NOT_PREVENTED,
|
|
||||||
PHASE_COMMIT,
|
|
||||||
PHASE_CREATE_PR,
|
|
||||||
PHASE_LOCK,
|
|
||||||
assess_work_issue_duplicate_gate,
|
|
||||||
assess_work_issue_duplicate_report,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
class TestDuplicateGateAssessment(unittest.TestCase):
|
|
||||||
def test_clear_issue_passes(self):
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
400,
|
|
||||||
open_prs=[],
|
|
||||||
branch_names=["feat/other-issue-99"],
|
|
||||||
claim_entry={"status": "not_claimed"},
|
|
||||||
locked_branch="feat/issue-400-duplicate-work-preflight",
|
|
||||||
phase=PHASE_LOCK,
|
|
||||||
)
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
|
|
||||||
|
|
||||||
def test_open_pr_blocks(self):
|
|
||||||
prs = [{
|
|
||||||
"number": 397,
|
|
||||||
"title": "feat: handoff",
|
|
||||||
"body": "Closes #395",
|
|
||||||
"head": {"ref": "feat/issue-395-proof-backed-review-handoff"},
|
|
||||||
}]
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
395,
|
|
||||||
open_prs=prs,
|
|
||||||
branch_names=[],
|
|
||||||
phase=PHASE_LOCK,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_PR_PREVENTED)
|
|
||||||
|
|
||||||
def test_conflicting_remote_branch_blocks(self):
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
395,
|
|
||||||
open_prs=[],
|
|
||||||
branch_names=["feat/issue-395-proof-backed-handoff-claims"],
|
|
||||||
locked_branch="feat/issue-395-new-attempt",
|
|
||||||
phase=PHASE_LOCK,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_BRANCH_PREVENTED)
|
|
||||||
|
|
||||||
def test_active_claim_on_other_branch_blocks(self):
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
398,
|
|
||||||
open_prs=[],
|
|
||||||
branch_names=[],
|
|
||||||
claim_entry={
|
|
||||||
"status": "active",
|
|
||||||
"latest_heartbeat": {
|
|
||||||
"branch": "feat/issue-398-validation-cwd-proof",
|
|
||||||
},
|
|
||||||
},
|
|
||||||
locked_branch="feat/issue-398-other-branch",
|
|
||||||
phase=PHASE_LOCK,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_commit_phase_maps_to_commit_outcome(self):
|
|
||||||
prs = [{
|
|
||||||
"number": 411,
|
|
||||||
"title": "x",
|
|
||||||
"body": "Closes #398",
|
|
||||||
"head": {"ref": "feat/issue-398-validation-cwd-proof"},
|
|
||||||
}]
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
398,
|
|
||||||
open_prs=prs,
|
|
||||||
branch_names=[],
|
|
||||||
locked_branch="feat/issue-398-alt",
|
|
||||||
phase=PHASE_COMMIT,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertEqual(result["outcome"], OUTCOME_DUPLICATE_COMMIT_PREVENTED)
|
|
||||||
|
|
||||||
def test_stale_claim_does_not_block_by_status_alone(self):
|
|
||||||
result = assess_work_issue_duplicate_gate(
|
|
||||||
400,
|
|
||||||
open_prs=[],
|
|
||||||
branch_names=[],
|
|
||||||
claim_entry={"status": "reclaimable", "reasons": ["stale"]},
|
|
||||||
locked_branch="feat/issue-400-duplicate-work-preflight",
|
|
||||||
phase=PHASE_LOCK,
|
|
||||||
)
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
|
|
||||||
|
|
||||||
class TestDuplicateReportOutcome(unittest.TestCase):
|
|
||||||
def test_requires_exactly_one_outcome(self):
|
|
||||||
bad = assess_work_issue_duplicate_report("work finished")
|
|
||||||
self.assertFalse(bad["complete"])
|
|
||||||
|
|
||||||
good = assess_work_issue_duplicate_report(
|
|
||||||
"Duplicate work not prevented for issue #400."
|
|
||||||
)
|
|
||||||
self.assertTrue(good["complete"])
|
|
||||||
self.assertEqual(good["outcome"], OUTCOME_DUPLICATE_WORK_NOT_PREVENTED)
|
|
||||||
|
|
||||||
|
|
||||||
class TestInjectableDuplicateFetcher(unittest.TestCase):
|
|
||||||
@patch("mcp_server.api_get_all", return_value=[])
|
|
||||||
@patch("mcp_server.get_auth_header", return_value="token x")
|
|
||||||
def test_lock_issue_uses_injected_fetcher(self, _auth, _api):
|
|
||||||
seen = {}
|
|
||||||
|
|
||||||
def fetcher(h, o, r, auth, issue_number):
|
|
||||||
seen["issue_number"] = issue_number
|
|
||||||
return [], [], {"status": "not_claimed"}
|
|
||||||
|
|
||||||
with patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
side_effect=fetcher,
|
|
||||||
), patch(
|
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
|
||||||
return_value={
|
|
||||||
"current_branch": "master",
|
|
||||||
"porcelain_status": "",
|
|
||||||
"base_equivalent": True,
|
|
||||||
},
|
|
||||||
):
|
|
||||||
with tempfile.TemporaryDirectory() as lock_dir:
|
|
||||||
with patch.dict(os.environ, {
|
|
||||||
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment",
|
|
||||||
"GITEA_ISSUE_LOCK_DIR": lock_dir,
|
|
||||||
}, clear=True):
|
|
||||||
mcp_server.gitea_lock_issue(
|
|
||||||
issue_number=400,
|
|
||||||
branch_name="feat/issue-400-duplicate-work-preflight",
|
|
||||||
remote="prgs",
|
|
||||||
)
|
|
||||||
self.assertEqual(seen["issue_number"], 400)
|
|
||||||
|
|
||||||
|
|
||||||
class TestMcpDuplicateRecheck(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
self._dir = tempfile.TemporaryDirectory()
|
|
||||||
self._env_patch = patch.dict(
|
|
||||||
os.environ,
|
|
||||||
{"GITEA_ISSUE_LOCK_DIR": self._dir.name},
|
|
||||||
clear=False,
|
|
||||||
)
|
|
||||||
self._env_patch.start()
|
|
||||||
self._remotes = patch.dict(mcp_server.REMOTES, {
|
|
||||||
"prgs": {"host": "gitea.example.com", "org": "Example-Org",
|
|
||||||
"repo": "Example-Repo"},
|
|
||||||
})
|
|
||||||
self._remotes.start()
|
|
||||||
mcp_server._IDENTITY_CACHE.clear()
|
|
||||||
|
|
||||||
def tearDown(self):
|
|
||||||
patch.stopall()
|
|
||||||
self._dir.cleanup()
|
|
||||||
|
|
||||||
def _write_lock(self, issue_number=400, branch="feat/issue-400-x"):
|
|
||||||
worktree_path = os.path.realpath(os.getcwd())
|
|
||||||
work_lease = {
|
|
||||||
"operation_type": "author_issue_work",
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"branch": branch,
|
|
||||||
"claimant": {"username": "test-user", "profile": "test-author"},
|
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
|
||||||
}
|
|
||||||
issue_lock_store.bind_session_lock({
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"branch_name": branch,
|
|
||||||
"remote": "prgs",
|
|
||||||
"org": "Example-Org",
|
|
||||||
"repo": "Example-Repo",
|
|
||||||
"worktree_path": worktree_path,
|
|
||||||
"work_lease": work_lease,
|
|
||||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
|
||||||
tool="gitea_lock_issue",
|
|
||||||
claimant=work_lease.get("claimant"),
|
|
||||||
),
|
|
||||||
})
|
|
||||||
|
|
||||||
@patch("mcp_server._assess_issue_duplicate_gate")
|
|
||||||
@patch("mcp_server.get_profile", return_value={
|
|
||||||
"profile_name": "test-author",
|
|
||||||
"allowed_operations": ["gitea.read", "gitea.repo.commit"],
|
|
||||||
"forbidden_operations": [],
|
|
||||||
"audit_label": "test-author",
|
|
||||||
})
|
|
||||||
@patch("mcp_server.get_auth_header", return_value="token x")
|
|
||||||
def test_commit_files_blocked_on_recheck(self, _auth, _profile, mock_gate):
|
|
||||||
self._write_lock()
|
|
||||||
mock_gate.return_value = {
|
|
||||||
"block": True,
|
|
||||||
"reasons": ["open PR #412 already covers issue #400"],
|
|
||||||
"outcome": OUTCOME_DUPLICATE_COMMIT_PREVENTED,
|
|
||||||
"safe_next_action": "stop",
|
|
||||||
}
|
|
||||||
mcp_server.record_preflight_check("whoami")
|
|
||||||
mcp_server.record_preflight_check("capability", resolved_role="author")
|
|
||||||
with patch(
|
|
||||||
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
|
||||||
return_value=(True, []),
|
|
||||||
):
|
|
||||||
result = mcp_server.gitea_commit_files(
|
|
||||||
files=[{
|
|
||||||
"operation": "create",
|
|
||||||
"path": "a.txt",
|
|
||||||
"content_plain": "hi",
|
|
||||||
}],
|
|
||||||
message="test",
|
|
||||||
remote="prgs",
|
|
||||||
)
|
|
||||||
self.assertFalse(result["success"])
|
|
||||||
self.assertIn("duplicate_gate", result)
|
|
||||||
|
|
||||||
@patch("mcp_server._assess_issue_duplicate_gate")
|
|
||||||
@patch("mcp_server.get_profile", return_value={
|
|
||||||
"profile_name": "test-author",
|
|
||||||
"allowed_operations": ["gitea.read", "gitea.pr.create"],
|
|
||||||
"forbidden_operations": [],
|
|
||||||
"audit_label": "test-author",
|
|
||||||
})
|
|
||||||
@patch("mcp_server.get_auth_header", return_value="token x")
|
|
||||||
def test_create_pr_returns_handoff_on_duplicate(self, _auth, _profile, mock_gate):
|
|
||||||
self._write_lock()
|
|
||||||
mock_gate.return_value = {
|
|
||||||
"block": True,
|
|
||||||
"reasons": ["open PR #412 already covers issue #400"],
|
|
||||||
"outcome": OUTCOME_DUPLICATE_PR_PREVENTED,
|
|
||||||
"safe_next_action": "reconciliation handoff",
|
|
||||||
}
|
|
||||||
mcp_server.record_preflight_check("whoami")
|
|
||||||
mcp_server.record_preflight_check("capability", resolved_role="author")
|
|
||||||
with patch(
|
|
||||||
"mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
|
||||||
return_value=(True, []),
|
|
||||||
):
|
|
||||||
result = mcp_server.gitea_create_pr(
|
|
||||||
title="feat: x (Closes #400)",
|
|
||||||
head="feat/issue-400-x",
|
|
||||||
base="master",
|
|
||||||
body="Closes #400",
|
|
||||||
remote="prgs",
|
|
||||||
worktree_path=os.path.realpath(os.getcwd()),
|
|
||||||
)
|
|
||||||
self.assertFalse(result["success"])
|
|
||||||
self.assertIsNone(result.get("number"))
|
|
||||||
self.assertIn("duplicate_gate", result)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
+101
-393
@@ -6,7 +6,6 @@ the MCP protocol) with mocked API responses.
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
@@ -46,7 +45,6 @@ from gitea_auth import get_profile # noqa: E402
|
|||||||
import gitea_config # noqa: E402
|
import gitea_config # noqa: E402
|
||||||
|
|
||||||
import mcp_server
|
import mcp_server
|
||||||
import issue_lock_store
|
|
||||||
|
|
||||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||||
|
|
||||||
@@ -81,64 +79,6 @@ def _visible_approval_reviews(reviewer="reviewer-bot", sha="abc123"):
|
|||||||
return [_formal_review(reviewer, "APPROVED", sha=sha)]
|
return [_formal_review(reviewer, "APPROVED", sha=sha)]
|
||||||
|
|
||||||
|
|
||||||
_DEFAULT_LEASE_SESSION = "mcp-test-reviewer-lease"
|
|
||||||
|
|
||||||
|
|
||||||
def _reviewer_lease_comment(
|
|
||||||
pr_number,
|
|
||||||
*,
|
|
||||||
session_id=_DEFAULT_LEASE_SESSION,
|
|
||||||
head_sha="abc123",
|
|
||||||
reviewer="reviewer-bot",
|
|
||||||
):
|
|
||||||
from datetime import datetime, timezone
|
|
||||||
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
body = reviewer_pr_lease.format_lease_body(
|
|
||||||
repo="Scaled-Tech-Consulting/Gitea-Tools",
|
|
||||||
pr_number=pr_number,
|
|
||||||
issue_number=407,
|
|
||||||
reviewer_identity=reviewer,
|
|
||||||
profile="gitea-reviewer",
|
|
||||||
session_id=session_id,
|
|
||||||
worktree="branches/review-test",
|
|
||||||
phase="claimed",
|
|
||||||
candidate_head=head_sha,
|
|
||||||
target_branch="master",
|
|
||||||
target_branch_sha="b" * 40,
|
|
||||||
last_activity=datetime.now(timezone.utc),
|
|
||||||
)
|
|
||||||
return {"id": 9001, "body": body, "user": {"login": reviewer}}
|
|
||||||
|
|
||||||
|
|
||||||
def _install_owned_reviewer_lease(
|
|
||||||
pr_number,
|
|
||||||
*,
|
|
||||||
session_id=_DEFAULT_LEASE_SESSION,
|
|
||||||
head_sha="abc123",
|
|
||||||
):
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
reviewer_pr_lease.clear_session_lease()
|
|
||||||
reviewer_pr_lease.record_session_lease({
|
|
||||||
"pr_number": pr_number,
|
|
||||||
"session_id": session_id,
|
|
||||||
"candidate_head": head_sha,
|
|
||||||
"target_branch": "master",
|
|
||||||
})
|
|
||||||
return patch(
|
|
||||||
"mcp_server._fetch_pr_comments",
|
|
||||||
return_value=[
|
|
||||||
_reviewer_lease_comment(
|
|
||||||
pr_number,
|
|
||||||
session_id=session_id,
|
|
||||||
head_sha=head_sha,
|
|
||||||
)
|
|
||||||
],
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
# Issue-write tools are profile-gated (#69).
|
# Issue-write tools are profile-gated (#69).
|
||||||
ISSUE_WRITE_ENV = {
|
ISSUE_WRITE_ENV = {
|
||||||
"GITEA_ALLOWED_OPERATIONS": (
|
"GITEA_ALLOWED_OPERATIONS": (
|
||||||
@@ -161,48 +101,17 @@ ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"
|
|||||||
|
|
||||||
|
|
||||||
def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
|
def _sample_issue_lock(issue_number=123, branch_name="feat/x", **overrides):
|
||||||
import issue_lock_provenance
|
|
||||||
|
|
||||||
work_lease = {
|
|
||||||
"operation_type": "author_issue_work",
|
|
||||||
"issue_number": issue_number,
|
|
||||||
"branch": branch_name,
|
|
||||||
"claimant": {"username": "test-user", "profile": "test-author"},
|
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
|
||||||
}
|
|
||||||
record = {
|
record = {
|
||||||
"issue_number": issue_number,
|
"issue_number": issue_number,
|
||||||
"branch_name": branch_name,
|
"branch_name": branch_name,
|
||||||
"remote": "dadeschools",
|
"remote": "dadeschools",
|
||||||
"org": "Scaled-Tech-Consulting",
|
"org": "Scaled-Tech-Consulting",
|
||||||
"repo": "Gitea-Tools",
|
"repo": "Gitea-Tools",
|
||||||
"worktree_path": "/tmp/test-worktree",
|
|
||||||
"work_lease": work_lease,
|
|
||||||
"lock_provenance": issue_lock_provenance.build_sanctioned_lock_provenance(
|
|
||||||
tool="gitea_lock_issue",
|
|
||||||
claimant=work_lease.get("claimant"),
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
record.update(overrides)
|
record.update(overrides)
|
||||||
return record
|
return record
|
||||||
|
|
||||||
|
|
||||||
def _clear_duplicate_context_fetcher(*_args, **_kwargs):
|
|
||||||
"""Default injectable duplicate-work context for lock/create_pr tests."""
|
|
||||||
return [], [], {"status": "not_claimed"}
|
|
||||||
|
|
||||||
|
|
||||||
def _bind_test_lock(**overrides) -> str:
|
|
||||||
remote = overrides.get("remote", "dadeschools")
|
|
||||||
record = _sample_issue_lock(**overrides)
|
|
||||||
if remote in mcp_server.REMOTES:
|
|
||||||
profile = mcp_server.REMOTES[remote]
|
|
||||||
record.setdefault("org", profile["org"])
|
|
||||||
record.setdefault("repo", profile["repo"])
|
|
||||||
record["remote"] = remote
|
|
||||||
return issue_lock_store.bind_session_lock(record)
|
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Create Issue
|
# Create Issue
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -257,78 +166,55 @@ class TestCreateIssue(unittest.TestCase):
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
class TestCreatePR(unittest.TestCase):
|
class TestCreatePR(unittest.TestCase):
|
||||||
|
|
||||||
@patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_creates_pr(self, _auth, mock_api, _role, _dup_fetcher):
|
@patch("os.path.exists", return_value=True)
|
||||||
worktree = os.path.realpath(os.getcwd())
|
@patch("builtins.open")
|
||||||
|
def test_creates_pr(self, mock_open, mock_exists, _auth, mock_api, _role):
|
||||||
|
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||||
|
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||||
with tempfile.TemporaryDirectory() as lock_dir:
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main")
|
||||||
with patch.dict(os.environ, env, clear=True):
|
|
||||||
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
|
|
||||||
result = gitea_create_pr(
|
|
||||||
title="feat: X Closes #123",
|
|
||||||
head="feat/x",
|
|
||||||
base="main",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertEqual(result["number"], 3)
|
self.assertEqual(result["number"], 3)
|
||||||
self.assertNotIn("url", result)
|
self.assertNotIn("url", result)
|
||||||
|
mock_exists.assert_called_with(ISSUE_LOCK_FILE)
|
||||||
|
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
|
||||||
payload = mock_api.call_args[0][3]
|
payload = mock_api.call_args[0][3]
|
||||||
self.assertEqual(payload["head"], "feat/x")
|
self.assertEqual(payload["head"], "feat/x")
|
||||||
self.assertEqual(payload["base"], "main")
|
self.assertEqual(payload["base"], "main")
|
||||||
self.assertIn("Closes #123", payload["title"])
|
self.assertIn("Closes #123", payload["title"])
|
||||||
|
|
||||||
@patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_reveal_opt_in_includes_url(self, _auth, mock_api, _role, _dup_fetcher):
|
@patch("os.path.exists", return_value=True)
|
||||||
worktree = os.path.realpath(os.getcwd())
|
@patch("builtins.open")
|
||||||
|
def test_create_pr_reveal_opt_in_includes_url(self, mock_open, mock_exists, _auth, mock_api, _role):
|
||||||
|
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||||
|
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||||
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
mock_api.return_value = {"number": 3, "html_url": "https://example.com/pulls/3"}
|
||||||
with tempfile.TemporaryDirectory() as lock_dir:
|
env = {**CREATE_PR_ENV, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
|
||||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir, "GITEA_MCP_REVEAL_ENDPOINTS": "1"}
|
|
||||||
with patch.dict(os.environ, env, clear=True):
|
with patch.dict(os.environ, env, clear=True):
|
||||||
_bind_test_lock(issue_number=123, branch_name="feat/x", worktree_path=worktree)
|
result = gitea_create_pr(title="feat: X Closes #123", head="feat/x", base="main")
|
||||||
result = gitea_create_pr(
|
|
||||||
title="feat: X Closes #123",
|
|
||||||
head="feat/x",
|
|
||||||
base="main",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("pulls/3", result["url"])
|
self.assertIn("pulls/3", result["url"])
|
||||||
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_locked_issue_mismatch_fails(self, _auth, _role):
|
@patch("os.path.exists", return_value=True)
|
||||||
worktree = os.path.realpath(os.getcwd())
|
@patch("builtins.open")
|
||||||
with tempfile.TemporaryDirectory() as lock_dir:
|
def test_create_pr_locked_issue_mismatch_fails(self, mock_open, mock_exists, _auth, _role):
|
||||||
env = {**CREATE_PR_ENV, "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
lock_json = json.dumps(_sample_issue_lock(issue_number=123, branch_name="feat/x"))
|
||||||
with patch.dict(os.environ, env, clear=True):
|
mock_open.return_value.__enter__.return_value.read.return_value = lock_json
|
||||||
_bind_test_lock(
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
issue_number=123,
|
|
||||||
branch_name="feat/x",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_create_pr(
|
gitea_create_pr(title="feat: X Closes #999", head="feat/x", base="main")
|
||||||
title="feat: X Closes #999",
|
|
||||||
head="feat/x",
|
|
||||||
base="main",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("Closes #123", str(ctx.exception))
|
self.assertIn("Closes #123", str(ctx.exception))
|
||||||
|
mock_open.assert_called_with(ISSUE_LOCK_FILE, "r", encoding="utf-8")
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -626,19 +512,6 @@ class TestViewPR(unittest.TestCase):
|
|||||||
class TestMergePR(unittest.TestCase):
|
class TestMergePR(unittest.TestCase):
|
||||||
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
|
"""Gated merge workflow (#16). gitea_merge_pr is the only merge path."""
|
||||||
|
|
||||||
def setUp(self):
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
self._lease_patch = _install_owned_reviewer_lease(8)
|
|
||||||
self._lease_patch.start()
|
|
||||||
self._auth_identity_patch = patch(
|
|
||||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
|
||||||
)
|
|
||||||
self._auth_identity_patch.start()
|
|
||||||
self.addCleanup(self._auth_identity_patch.stop)
|
|
||||||
self.addCleanup(self._lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||||
return {
|
return {
|
||||||
"user": {"login": author},
|
"user": {"login": author},
|
||||||
@@ -899,11 +772,9 @@ class TestMergePR(unittest.TestCase):
|
|||||||
pr_number=8, confirmation=self._confirm(8),
|
pr_number=8, confirmation=self._confirm(8),
|
||||||
expected_head_sha="deadbeef", remote="prgs")
|
expected_head_sha="deadbeef", remote="prgs")
|
||||||
self.assertFalse(r["performed"])
|
self.assertFalse(r["performed"])
|
||||||
self.assertTrue(any(
|
self.assertIn(
|
||||||
"expected head SHA does not match current PR head (fail closed)" in reason
|
"expected head SHA does not match current PR head (fail closed)",
|
||||||
or "PR head changed during lease" in reason
|
r["reasons"])
|
||||||
for reason in r["reasons"]
|
|
||||||
))
|
|
||||||
self._assert_no_merge_call(mock_api)
|
self._assert_no_merge_call(mock_api)
|
||||||
|
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@@ -1782,20 +1653,7 @@ class TestReviewDecisionValidationGate(unittest.TestCase):
|
|||||||
}
|
}
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
init_review_decision_lock("prgs", "review_pr")
|
init_review_decision_lock("prgs", "review_pr")
|
||||||
self._lease_patch = _install_owned_reviewer_lease(
|
|
||||||
self.PR, head_sha=self.SHA,
|
|
||||||
)
|
|
||||||
self._lease_patch.start()
|
|
||||||
self._auth_identity_patch = patch(
|
|
||||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
|
||||||
)
|
|
||||||
self._auth_identity_patch.start()
|
|
||||||
self.addCleanup(self._auth_identity_patch.stop)
|
|
||||||
self.addCleanup(self._lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def _env(self):
|
def _env(self):
|
||||||
return patch.dict(os.environ, {
|
return patch.dict(os.environ, {
|
||||||
@@ -1890,19 +1748,8 @@ class TestSubmitPrReview(unittest.TestCase):
|
|||||||
"""Gated review-mutation tool (#15)."""
|
"""Gated review-mutation tool (#15)."""
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
init_review_decision_lock("prgs", "review_pr")
|
init_review_decision_lock("prgs", "review_pr")
|
||||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
||||||
self._lease_patch = _install_owned_reviewer_lease(8)
|
|
||||||
self._lease_patch.start()
|
|
||||||
self._auth_identity_patch = patch(
|
|
||||||
"mcp_server._authenticated_username", return_value="reviewer-bot"
|
|
||||||
)
|
|
||||||
self._auth_identity_patch.start()
|
|
||||||
self.addCleanup(self._auth_identity_patch.stop)
|
|
||||||
self.addCleanup(self._lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
def _pr(self, author, state="open", sha="abc123", mergeable=True):
|
||||||
return {
|
return {
|
||||||
@@ -2140,11 +1987,9 @@ class TestSubmitPrReview(unittest.TestCase):
|
|||||||
final_review_decision_ready=True,
|
final_review_decision_ready=True,
|
||||||
)
|
)
|
||||||
self.assertFalse(r["performed"])
|
self.assertFalse(r["performed"])
|
||||||
self.assertTrue(any(
|
self.assertIn(
|
||||||
"expected head SHA does not match current PR head (fail closed)" in reason
|
"expected head SHA does not match current PR head (fail closed)",
|
||||||
or "PR head changed during lease" in reason
|
r["reasons"])
|
||||||
for reason in r["reasons"]
|
|
||||||
))
|
|
||||||
self._assert_no_mutation(mock_api)
|
self._assert_no_mutation(mock_api)
|
||||||
|
|
||||||
def test_head_sha_match_allows(self):
|
def test_head_sha_match_allows(self):
|
||||||
@@ -2207,9 +2052,9 @@ class TestSubmitPrReview(unittest.TestCase):
|
|||||||
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
env = {"GITEA_PROFILE_NAME": "gitea-reviewer",
|
||||||
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
"GITEA_ALLOWED_OPERATIONS": "read,review,approve"}
|
||||||
with patch.dict(os.environ, env, clear=True):
|
with patch.dict(os.environ, env, clear=True):
|
||||||
gitea_mark_final_review_decision(8, "approve", remote="prgs")
|
gitea_mark_final_review_decision(5, "approve", remote="prgs")
|
||||||
r = gitea_submit_pr_review(
|
r = gitea_submit_pr_review(
|
||||||
pr_number=8, action="approve", remote="prgs",
|
pr_number=5, action="approve", remote="prgs",
|
||||||
final_review_decision_ready=True,
|
final_review_decision_ready=True,
|
||||||
)
|
)
|
||||||
self.assertFalse(r["performed"])
|
self.assertFalse(r["performed"])
|
||||||
@@ -2428,13 +2273,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
|||||||
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
||||||
|
|
||||||
def test_merge_pr_with_closes_removes_label(self):
|
def test_merge_pr_with_closes_removes_label(self):
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
|
|
||||||
lease_patch.start()
|
|
||||||
self.addCleanup(lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def api_side_effect(method, url, auth, payload=None):
|
def api_side_effect(method, url, auth, payload=None):
|
||||||
if method == "GET" and "/user" in url:
|
if method == "GET" and "/user" in url:
|
||||||
return {"login": "merger"}
|
return {"login": "merger"}
|
||||||
@@ -2469,13 +2307,6 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
|||||||
self.assertEqual(res["cleanup_status"].get(123), "released")
|
self.assertEqual(res["cleanup_status"].get(123), "released")
|
||||||
|
|
||||||
def test_merge_pr_with_branch_name_removes_label(self):
|
def test_merge_pr_with_branch_name_removes_label(self):
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
lease_patch = _install_owned_reviewer_lease(1, head_sha="sha123")
|
|
||||||
lease_patch.start()
|
|
||||||
self.addCleanup(lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
|
|
||||||
def api_side_effect(method, url, auth, payload=None):
|
def api_side_effect(method, url, auth, payload=None):
|
||||||
if method == "GET" and "/user" in url:
|
if method == "GET" and "/user" in url:
|
||||||
return {"login": "merger"}
|
return {"login": "merger"}
|
||||||
@@ -3157,8 +2988,6 @@ class TestVerifyMutationAuthority(unittest.TestCase):
|
|||||||
# profile; the active profile resolves as reviewer — side-channel
|
# profile; the active profile resolves as reviewer — side-channel
|
||||||
# override rejected even with a matching in-process authority.
|
# override rejected even with a matching in-process authority.
|
||||||
self._authority()
|
self._authority()
|
||||||
with patch("mcp_server.gitea_config.is_runtime_switching_enabled",
|
|
||||||
return_value=False):
|
|
||||||
with patch.dict(os.environ,
|
with patch.dict(os.environ,
|
||||||
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
|
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
@@ -3213,37 +3042,22 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
"""Test issue locking and PR gating constraints."""
|
"""Test issue locking and PR gating constraints."""
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
self._lock_dir = tempfile.TemporaryDirectory()
|
self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True)
|
||||||
env = {
|
|
||||||
**ISSUE_WRITE_ENV,
|
|
||||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
|
||||||
}
|
|
||||||
self._env_patcher = patch.dict(os.environ, env, clear=True)
|
|
||||||
self._env_patcher.start()
|
self._env_patcher.start()
|
||||||
self._dup_fetcher_patcher = patch(
|
|
||||||
"mcp_server.issue_duplicate_context_fetcher",
|
|
||||||
return_value=([], [], {"status": "not_claimed"}),
|
|
||||||
)
|
|
||||||
self.mock_dup_fetcher = self._dup_fetcher_patcher.start()
|
|
||||||
|
|
||||||
def tearDown(self):
|
def tearDown(self):
|
||||||
self._dup_fetcher_patcher.stop()
|
|
||||||
self._env_patcher.stop()
|
self._env_patcher.stop()
|
||||||
self._lock_dir.cleanup()
|
if os.path.exists(ISSUE_LOCK_FILE):
|
||||||
|
os.remove(ISSUE_LOCK_FILE)
|
||||||
def _create_pr_env(self) -> dict:
|
|
||||||
return {
|
|
||||||
**CREATE_PR_ENV,
|
|
||||||
"GITEA_ISSUE_LOCK_DIR": self._lock_dir.name,
|
|
||||||
}
|
|
||||||
|
|
||||||
@patch(
|
@patch(
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
return_value=_clean_master_git_state_for_lock(),
|
||||||
)
|
)
|
||||||
@patch("mcp_server.api_get_all", return_value=[])
|
@patch("mcp_server.api_get_all")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_lock_issue_success(self, _auth, _api, _git_state):
|
def test_lock_issue_success(self, _auth, mock_api, _git_state):
|
||||||
|
mock_api.return_value = [] # no open PRs
|
||||||
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
res = gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
self.assertTrue(res["success"])
|
self.assertTrue(res["success"])
|
||||||
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
|
self.assertEqual(res["work_lease"]["operation_type"], "author_issue_work")
|
||||||
@@ -3253,8 +3067,9 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
self.assertIn("expires_at", res["work_lease"])
|
self.assertIn("expires_at", res["work_lease"])
|
||||||
self.assertIn("last_heartbeat_at", res["work_lease"])
|
self.assertIn("last_heartbeat_at", res["work_lease"])
|
||||||
self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default")
|
self.assertEqual(res["work_lease"]["claimant"]["profile"], "gitea-default")
|
||||||
self.assertIn("lock_file_path", res)
|
self.assertTrue(os.path.exists(ISSUE_LOCK_FILE))
|
||||||
lock = issue_lock_store.read_lock_file(res["lock_file_path"])
|
with open(ISSUE_LOCK_FILE, encoding="utf-8") as f:
|
||||||
|
lock = json.load(f)
|
||||||
self.assertIn("worktree_path", lock)
|
self.assertIn("worktree_path", lock)
|
||||||
self.assertIn("work_lease", lock)
|
self.assertIn("work_lease", lock)
|
||||||
|
|
||||||
@@ -3267,48 +3082,18 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
return_value=_clean_master_git_state_for_lock(),
|
||||||
)
|
)
|
||||||
|
@patch("mcp_server.api_get_all")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_lock_issue_reused_by_open_pr_branch(self, _auth, _git_state):
|
def test_lock_issue_reused_by_open_pr_branch(self, _auth, mock_api, _git_state):
|
||||||
self.mock_dup_fetcher.return_value = ([{
|
mock_api.return_value = [{
|
||||||
"number": 200,
|
"number": 200,
|
||||||
"head": {"ref": "feat/issue-196-boundary"},
|
"head": {"ref": "feat/issue-196-boundary"},
|
||||||
"title": "Some PR",
|
"title": "Some PR",
|
||||||
"body": "No closes ref",
|
"body": "No closes ref"
|
||||||
}], [], {"status": "not_claimed"})
|
}]
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||||
|
|
||||||
@patch(
|
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
|
||||||
)
|
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
|
||||||
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, _git_state):
|
|
||||||
self.mock_dup_fetcher.return_value = ([{
|
|
||||||
"number": 200,
|
|
||||||
"head": {"ref": "feat/other-branch"},
|
|
||||||
"title": "Some PR",
|
|
||||||
"body": "fixes #196",
|
|
||||||
}], [], {"status": "not_claimed"})
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
|
||||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
|
||||||
self.assertIn("open PR #200 already covers issue", str(ctx.exception))
|
|
||||||
|
|
||||||
@patch(
|
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
|
||||||
)
|
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
|
||||||
def test_lock_issue_reused_by_remote_branch(self, _auth, _git_state):
|
|
||||||
self.mock_dup_fetcher.return_value = (
|
|
||||||
[],
|
|
||||||
["feat/issue-196-existing-work"],
|
|
||||||
{"status": "not_claimed"},
|
|
||||||
)
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
|
||||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
|
||||||
self.assertIn("remote branch(es) already match issue pattern", str(ctx.exception))
|
|
||||||
|
|
||||||
@patch(
|
@patch(
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
@@ -3316,75 +3101,58 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
@patch("mcp_server.api_get_all")
|
@patch("mcp_server.api_get_all")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_lock_issue_adopts_exact_own_branch(self, _auth, mock_api, _git_state):
|
def test_lock_issue_reused_by_open_pr_closes_ref(self, _auth, mock_api, _git_state):
|
||||||
branch = "feat/issue-196-mutations"
|
mock_api.return_value = [{
|
||||||
self.mock_dup_fetcher.return_value = ([], [branch], {"status": "not_claimed"})
|
"number": 200,
|
||||||
mock_api.return_value = [{"name": branch, "commit": {"id": "abc123"}}]
|
"head": {"ref": "feat/other-branch"},
|
||||||
res = gitea_lock_issue(issue_number=196, branch_name=branch, remote="prgs")
|
"title": "Some PR",
|
||||||
self.assertTrue(res["success"])
|
"body": "fixes #196"
|
||||||
self.assertIn("adoption", res)
|
}]
|
||||||
self.assertEqual(res["adoption"]["branch_head_commit"], "abc123")
|
with self.assertRaises(ValueError) as ctx:
|
||||||
|
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
|
self.assertIn("already tied to an open PR", str(ctx.exception))
|
||||||
|
|
||||||
@patch(
|
@patch(
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
return_value=_clean_master_git_state_for_lock(),
|
||||||
)
|
)
|
||||||
@patch("mcp_server.api_get_all", return_value=[])
|
@patch("mcp_server.api_get_all")
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_lock_issue_blocks_active_same_operation_lease(self, _auth, _api, _git_state):
|
def test_lock_issue_reused_by_remote_branch(self, _auth, mock_api, _git_state):
|
||||||
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
|
mock_api.side_effect = [
|
||||||
issue_lock_store.save_lock_file(
|
[],
|
||||||
issue_lock_store.lock_file_path(
|
[{"name": "feat/issue-196-existing-work"}],
|
||||||
remote="prgs",
|
]
|
||||||
org="Scaled-Tech-Consulting",
|
with self.assertRaises(ValueError) as ctx:
|
||||||
repo=prgs_repo,
|
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
issue_number=196,
|
self.assertIn("already has matching branch", str(ctx.exception))
|
||||||
),
|
|
||||||
{
|
def test_lock_issue_blocks_active_same_operation_lease(self):
|
||||||
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
|
json.dump({
|
||||||
"issue_number": 196,
|
"issue_number": 196,
|
||||||
"branch_name": "feat/issue-196-other-work",
|
"branch_name": "feat/issue-196-other-work",
|
||||||
"remote": "prgs",
|
|
||||||
"org": "Scaled-Tech-Consulting",
|
|
||||||
"repo": prgs_repo,
|
|
||||||
"worktree_path": "/tmp/other-worktree",
|
"worktree_path": "/tmp/other-worktree",
|
||||||
"work_lease": {
|
"work_lease": {
|
||||||
"operation_type": "author_issue_work",
|
"operation_type": "author_issue_work",
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
"expires_at": "2999-01-01T00:00:00Z",
|
||||||
},
|
},
|
||||||
},
|
}, f)
|
||||||
)
|
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
self.assertIn("already has an active author_issue_work lease", str(ctx.exception))
|
self.assertIn("already has an active author_issue_work lease", str(ctx.exception))
|
||||||
|
|
||||||
@patch(
|
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self):
|
||||||
"mcp_server.issue_lock_worktree.read_worktree_git_state",
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
return_value=_clean_master_git_state_for_lock(),
|
json.dump({
|
||||||
)
|
|
||||||
@patch("mcp_server.api_get_all", return_value=[])
|
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
|
||||||
def test_lock_issue_blocks_expired_same_operation_lease_for_recovery(self, _auth, _api, _git_state):
|
|
||||||
prgs_repo = mcp_server.REMOTES["prgs"]["repo"]
|
|
||||||
issue_lock_store.save_lock_file(
|
|
||||||
issue_lock_store.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo=prgs_repo,
|
|
||||||
issue_number=196,
|
|
||||||
),
|
|
||||||
{
|
|
||||||
"issue_number": 196,
|
"issue_number": 196,
|
||||||
"branch_name": "feat/issue-196-other-work",
|
"branch_name": "feat/issue-196-other-work",
|
||||||
"remote": "prgs",
|
|
||||||
"org": "Scaled-Tech-Consulting",
|
|
||||||
"repo": prgs_repo,
|
|
||||||
"worktree_path": "/tmp/other-worktree",
|
"worktree_path": "/tmp/other-worktree",
|
||||||
"work_lease": {
|
"work_lease": {
|
||||||
"operation_type": "author_issue_work",
|
"operation_type": "author_issue_work",
|
||||||
"expires_at": "2000-01-01T00:00:00Z",
|
"expires_at": "2000-01-01T00:00:00Z",
|
||||||
},
|
},
|
||||||
},
|
}, f)
|
||||||
)
|
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
gitea_lock_issue(issue_number=196, branch_name="feat/issue-196-mutations", remote="prgs")
|
||||||
self.assertIn("Recovery review is required before takeover", str(ctx.exception))
|
self.assertIn("Recovery review is required before takeover", str(ctx.exception))
|
||||||
@@ -3451,7 +3219,9 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_missing_lock_fails(self, _auth, _role):
|
def test_create_pr_missing_lock_fails(self, _auth, _role):
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
if os.path.exists(ISSUE_LOCK_FILE):
|
||||||
|
os.remove(ISSUE_LOCK_FILE)
|
||||||
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
with self.assertRaises(RuntimeError) as ctx:
|
||||||
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-mutations", remote="prgs")
|
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-mutations", remote="prgs")
|
||||||
self.assertIn("Issue lock is missing", str(ctx.exception))
|
self.assertIn("Issue lock is missing", str(ctx.exception))
|
||||||
@@ -3460,64 +3230,37 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_branch_mismatch_fails(self, _auth, _role):
|
def test_create_pr_branch_mismatch_fails(self, _auth, _role):
|
||||||
worktree = os.path.realpath(os.getcwd())
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
_bind_test_lock(
|
json.dump(_sample_issue_lock(
|
||||||
issue_number=196,
|
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||||
branch_name="feat/issue-196-mutations",
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_create_pr(
|
gitea_create_pr(title="feat: X Closes #196", head="feat/issue-196-different", remote="prgs")
|
||||||
title="feat: X Closes #196",
|
|
||||||
head="feat/issue-196-different",
|
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("does not match locked branch", str(ctx.exception))
|
self.assertIn("does not match locked branch", str(ctx.exception))
|
||||||
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_forbidden_terms_fails(self, _auth, _role):
|
def test_create_pr_forbidden_terms_fails(self, _auth, _role):
|
||||||
worktree = os.path.realpath(os.getcwd())
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
_bind_test_lock(
|
json.dump(_sample_issue_lock(
|
||||||
issue_number=196,
|
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||||
branch_name="feat/issue-196-mutations",
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
|
||||||
for term in ("equivalent to #196", "related to #196", "same as #196"):
|
for term in ("equivalent to #196", "related to #196", "same as #196"):
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_create_pr(
|
gitea_create_pr(title=f"feat: X {term}", head="feat/issue-196-mutations", remote="prgs")
|
||||||
title=f"feat: X {term}",
|
|
||||||
head="feat/issue-196-mutations",
|
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("contains forbidden term", str(ctx.exception))
|
self.assertIn("contains forbidden term", str(ctx.exception))
|
||||||
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_missing_closes_ref_fails(self, _auth, _role):
|
def test_create_pr_missing_closes_ref_fails(self, _auth, _role):
|
||||||
worktree = os.path.realpath(os.getcwd())
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
_bind_test_lock(
|
json.dump(_sample_issue_lock(
|
||||||
issue_number=196,
|
issue_number=196, branch_name="feat/issue-196-mutations"), f)
|
||||||
branch_name="feat/issue-196-mutations",
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_create_pr(
|
gitea_create_pr(title="feat: X refs #196", head="feat/issue-196-mutations", remote="prgs")
|
||||||
title="feat: X refs #196",
|
|
||||||
head="feat/issue-196-mutations",
|
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
|
self.assertIn("must contain 'Closes #196' or 'Fixes #196' exactly", str(ctx.exception))
|
||||||
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
@@ -3525,13 +3268,13 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
||||||
def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
|
def test_create_pr_worktree_mismatch_fails(self, _auth, _role):
|
||||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
|
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-pr")
|
||||||
_bind_test_lock(
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
|
json.dump(_sample_issue_lock(
|
||||||
issue_number=249,
|
issue_number=249,
|
||||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||||
worktree_path=scratch,
|
worktree_path=scratch,
|
||||||
remote="prgs",
|
), f)
|
||||||
)
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
|
||||||
with self.assertRaises(ValueError) as ctx:
|
with self.assertRaises(ValueError) as ctx:
|
||||||
gitea_create_pr(
|
gitea_create_pr(
|
||||||
title="feat: lock scratch worktree Closes #249",
|
title="feat: lock scratch worktree Closes #249",
|
||||||
@@ -3541,41 +3284,6 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertIn("does not match locked worktree", str(ctx.exception))
|
self.assertIn("does not match locked worktree", str(ctx.exception))
|
||||||
|
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
|
||||||
return_value=(True, []))
|
|
||||||
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
|
|
||||||
def test_create_pr_manual_lock_seed_blocked(self, _auth, _role):
|
|
||||||
worktree = os.path.realpath(os.getcwd())
|
|
||||||
with tempfile.TemporaryDirectory() as lock_dir:
|
|
||||||
env = {**self._create_pr_env(), "GITEA_ISSUE_LOCK_DIR": lock_dir}
|
|
||||||
with patch.dict(os.environ, env, clear=True):
|
|
||||||
issue_lock_store.save_lock_file(
|
|
||||||
issue_lock_store.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo=mcp_server.REMOTES["prgs"]["repo"],
|
|
||||||
issue_number=447,
|
|
||||||
lock_dir=lock_dir,
|
|
||||||
),
|
|
||||||
_sample_issue_lock(
|
|
||||||
issue_number=447,
|
|
||||||
branch_name="feat/issue-447-lock-provenance",
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo=mcp_server.REMOTES["prgs"]["repo"],
|
|
||||||
worktree_path=worktree,
|
|
||||||
lock_provenance=None,
|
|
||||||
),
|
|
||||||
)
|
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
|
||||||
gitea_create_pr(
|
|
||||||
title="feat: lock provenance Closes #447",
|
|
||||||
head="feat/issue-447-lock-provenance",
|
|
||||||
remote="prgs",
|
|
||||||
worktree_path=worktree,
|
|
||||||
)
|
|
||||||
self.assertIn("lock provenance", str(ctx.exception).lower())
|
|
||||||
|
|
||||||
@patch("mcp_server.api_request")
|
@patch("mcp_server.api_request")
|
||||||
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop",
|
||||||
return_value=(True, []))
|
return_value=(True, []))
|
||||||
@@ -3583,13 +3291,13 @@ class TestIssueLocking(unittest.TestCase):
|
|||||||
def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
|
def test_create_pr_honors_scratch_worktree_lock(self, _auth, _role, mock_api):
|
||||||
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
|
scratch = os.path.realpath("/tmp/gitea-tools-author-scratch/issue-249-e2e")
|
||||||
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
|
mock_api.return_value = {"number": 250, "html_url": "https://example/pr/250"}
|
||||||
_bind_test_lock(
|
with open(ISSUE_LOCK_FILE, "w", encoding="utf-8") as f:
|
||||||
|
json.dump(_sample_issue_lock(
|
||||||
issue_number=249,
|
issue_number=249,
|
||||||
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
branch_name="feat/issue-249-issue-lock-scratch-worktree",
|
||||||
worktree_path=scratch,
|
worktree_path=scratch,
|
||||||
remote="prgs",
|
), f)
|
||||||
)
|
with patch.dict(os.environ, CREATE_PR_ENV, clear=True):
|
||||||
with patch.dict(os.environ, self._create_pr_env(), clear=True):
|
|
||||||
res = gitea_create_pr(
|
res = gitea_create_pr(
|
||||||
title="feat: issue-lock scratch worktree Closes #249",
|
title="feat: issue-lock scratch worktree Closes #249",
|
||||||
head="feat/issue-249-issue-lock-scratch-worktree",
|
head="feat/issue-249-issue-lock-scratch-worktree",
|
||||||
|
|||||||
@@ -156,22 +156,9 @@ class TestPRQueueInventory(unittest.TestCase):
|
|||||||
]
|
]
|
||||||
|
|
||||||
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
from mcp_server import init_review_decision_lock, gitea_mark_final_review_decision
|
||||||
from tests.test_mcp_server import _install_owned_reviewer_lease
|
|
||||||
import reviewer_pr_lease
|
|
||||||
|
|
||||||
with patch("mcp_server._authenticated_username", return_value="reviewer1"):
|
|
||||||
init_review_decision_lock("prgs", "review_pr")
|
init_review_decision_lock("prgs", "review_pr")
|
||||||
gitea_mark_final_review_decision(1, "approve", remote="prgs")
|
gitea_mark_final_review_decision(1, "approve", remote="prgs")
|
||||||
lease_patch = _install_owned_reviewer_lease(
|
result = gitea_review_pr(pr_number=1, event="APPROVE", remote="prgs", final_review_decision_ready=True)
|
||||||
1, head_sha="abc1", session_id="inventory-review-lease",
|
|
||||||
)
|
|
||||||
lease_patch.start()
|
|
||||||
self.addCleanup(lease_patch.stop)
|
|
||||||
self.addCleanup(reviewer_pr_lease.clear_session_lease)
|
|
||||||
result = gitea_review_pr(
|
|
||||||
pr_number=1, event="APPROVE", remote="prgs",
|
|
||||||
final_review_decision_ready=True,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["success"])
|
self.assertTrue(result["success"])
|
||||||
self.assertIn("=== PR Queue Inventory ===", result["message"])
|
self.assertIn("=== PR Queue Inventory ===", result["message"])
|
||||||
self.assertIn("Repository:", result["message"])
|
self.assertIn("Repository:", result["message"])
|
||||||
|
|||||||
@@ -2346,7 +2346,6 @@ class TestWorkIssueFinalReport(unittest.TestCase):
|
|||||||
"- Safe next action: open PR",
|
"- Safe next action: open PR",
|
||||||
"- Next: open PR",
|
"- Next: open PR",
|
||||||
"- Safety statement: no review/merge",
|
"- Safety statement: no review/merge",
|
||||||
"- Duplicate work outcome: duplicate work not prevented",
|
|
||||||
])
|
])
|
||||||
|
|
||||||
def test_complete_work_issue_report_earns_a(self):
|
def test_complete_work_issue_report_earns_a(self):
|
||||||
|
|||||||
@@ -1,193 +0,0 @@
|
|||||||
"""Tests for per-PR reviewer leases (#407)."""
|
|
||||||
|
|
||||||
import sys
|
|
||||||
import unittest
|
|
||||||
from datetime import datetime, timedelta, timezone
|
|
||||||
from unittest.mock import patch
|
|
||||||
|
|
||||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import reviewer_pr_lease as leases
|
|
||||||
|
|
||||||
|
|
||||||
def _lease_comment(
|
|
||||||
pr_number: int,
|
|
||||||
session_id: str,
|
|
||||||
*,
|
|
||||||
phase: str = "claimed",
|
|
||||||
minutes_ago: int = 0,
|
|
||||||
candidate_head: str = "a" * 40,
|
|
||||||
) -> dict:
|
|
||||||
now = datetime.now(timezone.utc) - timedelta(minutes=minutes_ago)
|
|
||||||
body = leases.format_lease_body(
|
|
||||||
repo="Scaled-Tech-Consulting/Gitea-Tools",
|
|
||||||
pr_number=pr_number,
|
|
||||||
issue_number=295,
|
|
||||||
reviewer_identity="rev1",
|
|
||||||
profile="prgs-reviewer",
|
|
||||||
session_id=session_id,
|
|
||||||
worktree="branches/review-pr382",
|
|
||||||
phase=phase,
|
|
||||||
candidate_head=candidate_head,
|
|
||||||
target_branch="master",
|
|
||||||
target_branch_sha="b" * 40,
|
|
||||||
last_activity=now,
|
|
||||||
)
|
|
||||||
return {"id": 1, "body": body, "user": {"login": "rev1"}}
|
|
||||||
|
|
||||||
|
|
||||||
class TestReviewerLeaseAcquire(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
leases.clear_session_lease()
|
|
||||||
|
|
||||||
def test_two_reviewers_cannot_lease_same_pr(self):
|
|
||||||
comments = [_lease_comment(382, "session-a")]
|
|
||||||
result = leases.assess_acquire_lease(
|
|
||||||
comments,
|
|
||||||
pr_number=382,
|
|
||||||
reviewer_identity="rev2",
|
|
||||||
profile="prgs-reviewer",
|
|
||||||
session_id="session-b",
|
|
||||||
repo="Scaled-Tech-Consulting/Gitea-Tools",
|
|
||||||
issue_number=295,
|
|
||||||
worktree="branches/review-pr382-b",
|
|
||||||
candidate_head="c" * 40,
|
|
||||||
target_branch="master",
|
|
||||||
target_branch_sha="d" * 40,
|
|
||||||
)
|
|
||||||
self.assertFalse(result["acquire_allowed"])
|
|
||||||
self.assertTrue(any("already has active" in r for r in result["reasons"]))
|
|
||||||
|
|
||||||
def test_two_reviewers_can_lease_different_prs(self):
|
|
||||||
comments = [_lease_comment(382, "session-a")]
|
|
||||||
result = leases.assess_acquire_lease(
|
|
||||||
comments,
|
|
||||||
pr_number=383,
|
|
||||||
reviewer_identity="rev2",
|
|
||||||
profile="prgs-reviewer",
|
|
||||||
session_id="session-b",
|
|
||||||
repo="Scaled-Tech-Consulting/Gitea-Tools",
|
|
||||||
issue_number=296,
|
|
||||||
worktree="branches/review-pr383",
|
|
||||||
candidate_head="c" * 40,
|
|
||||||
target_branch="master",
|
|
||||||
target_branch_sha="d" * 40,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["acquire_allowed"])
|
|
||||||
self.assertIsNotNone(result["lease_body"])
|
|
||||||
|
|
||||||
|
|
||||||
class TestReviewerLeaseFreshness(unittest.TestCase):
|
|
||||||
def test_stale_warning_after_30_minutes(self):
|
|
||||||
lease = leases.parse_lease_comment(
|
|
||||||
_lease_comment(382, "session-a", minutes_ago=35)["body"]
|
|
||||||
)
|
|
||||||
self.assertEqual(
|
|
||||||
leases.classify_lease_freshness(lease),
|
|
||||||
"stale_warning",
|
|
||||||
)
|
|
||||||
|
|
||||||
def test_reclaimable_after_60_minutes(self):
|
|
||||||
lease = leases.parse_lease_comment(
|
|
||||||
_lease_comment(382, "session-a", minutes_ago=65)["body"]
|
|
||||||
)
|
|
||||||
self.assertEqual(
|
|
||||||
leases.classify_lease_freshness(lease),
|
|
||||||
"reclaimable",
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
class TestReviewerLeaseMutationGate(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
leases.clear_session_lease()
|
|
||||||
|
|
||||||
def test_reviewer_without_lease_cannot_mutate(self):
|
|
||||||
head = "f" * 40
|
|
||||||
comments = [_lease_comment(382, "other-session", candidate_head=head)]
|
|
||||||
result = leases.assess_mutation_lease_gate(
|
|
||||||
pr_number=382,
|
|
||||||
comments=comments,
|
|
||||||
reviewer_identity="rev1",
|
|
||||||
session_id="my-session",
|
|
||||||
mutation="approve",
|
|
||||||
live_head_sha=head,
|
|
||||||
pinned_head_sha=head,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
|
|
||||||
def test_owned_lease_allows_mutation(self):
|
|
||||||
head = "f" * 40
|
|
||||||
comments = [_lease_comment(382, "my-session", candidate_head=head)]
|
|
||||||
leases.record_session_lease({
|
|
||||||
"pr_number": 382,
|
|
||||||
"session_id": "my-session",
|
|
||||||
"candidate_head": head,
|
|
||||||
"target_branch": "master",
|
|
||||||
})
|
|
||||||
result = leases.assess_mutation_lease_gate(
|
|
||||||
pr_number=382,
|
|
||||||
comments=comments,
|
|
||||||
reviewer_identity="rev1",
|
|
||||||
session_id="my-session",
|
|
||||||
mutation="approve",
|
|
||||||
live_head_sha=head,
|
|
||||||
pinned_head_sha=head,
|
|
||||||
)
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
|
|
||||||
def test_head_change_invalidates_lease(self):
|
|
||||||
reviewed = "f" * 40
|
|
||||||
live = "e" * 40
|
|
||||||
comments = [_lease_comment(382, "my-session", candidate_head=reviewed)]
|
|
||||||
leases.record_session_lease({
|
|
||||||
"pr_number": 382,
|
|
||||||
"session_id": "my-session",
|
|
||||||
"candidate_head": reviewed,
|
|
||||||
})
|
|
||||||
result = leases.assess_mutation_lease_gate(
|
|
||||||
pr_number=382,
|
|
||||||
comments=comments,
|
|
||||||
reviewer_identity="rev1",
|
|
||||||
session_id="my-session",
|
|
||||||
mutation="merge",
|
|
||||||
live_head_sha=live,
|
|
||||||
pinned_head_sha=reviewed,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertTrue(any("head" in r.lower() for r in result["reasons"]))
|
|
||||||
|
|
||||||
|
|
||||||
class TestReviewerLeaseMcpGate(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
leases.clear_session_lease()
|
|
||||||
patch("mcp_server.verify_preflight_purity").start()
|
|
||||||
patch("gitea_audit.audit_enabled", return_value=False).start()
|
|
||||||
mcp_server = __import__("mcp_server")
|
|
||||||
mcp_server._IDENTITY_CACHE.clear()
|
|
||||||
mcp_server.init_review_decision_lock("prgs", "review_pr")
|
|
||||||
mcp_server.record_preflight_check("whoami")
|
|
||||||
mcp_server.record_preflight_check("capability", "reviewer")
|
|
||||||
|
|
||||||
def tearDown(self):
|
|
||||||
patch.stopall()
|
|
||||||
leases.clear_session_lease()
|
|
||||||
|
|
||||||
def test_reviewer_pr_lease_gate_helper_blocks_without_session(self):
|
|
||||||
import mcp_server
|
|
||||||
head = "a" * 40
|
|
||||||
with patch("mcp_server._fetch_pr_comments", return_value=[]):
|
|
||||||
reasons = mcp_server._reviewer_pr_lease_gate(
|
|
||||||
pr_number=382,
|
|
||||||
remote="prgs",
|
|
||||||
host=None,
|
|
||||||
org=None,
|
|
||||||
repo=None,
|
|
||||||
mutation="approve",
|
|
||||||
live_head_sha=head,
|
|
||||||
pinned_head_sha=head,
|
|
||||||
)
|
|
||||||
self.assertTrue(any("lease" in r.lower() for r in reasons))
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
@@ -12,14 +12,12 @@ from starlette.testclient import TestClient
|
|||||||
from webui.app import create_app
|
from webui.app import create_app
|
||||||
from webui.queue_loader import (
|
from webui.queue_loader import (
|
||||||
PaginationMeta,
|
PaginationMeta,
|
||||||
QueueSnapshot,
|
|
||||||
_classify_issue,
|
_classify_issue,
|
||||||
_classify_pr,
|
_classify_pr,
|
||||||
_extract_linked_issue,
|
_extract_linked_issue,
|
||||||
load_queue_snapshot,
|
load_queue_snapshot,
|
||||||
snapshot_to_dict,
|
snapshot_to_dict,
|
||||||
)
|
)
|
||||||
from webui.queue_views import render_queue_page
|
|
||||||
|
|
||||||
_RECENT = datetime.now(timezone.utc).isoformat()
|
_RECENT = datetime.now(timezone.utc).isoformat()
|
||||||
_STALE = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat()
|
_STALE = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat()
|
||||||
@@ -211,77 +209,36 @@ class TestQueueRoutes(unittest.TestCase):
|
|||||||
self.assertIsNotNone(snapshot.fetch_error)
|
self.assertIsNotNone(snapshot.fetch_error)
|
||||||
self.assertEqual(len(snapshot.prs), 0)
|
self.assertEqual(len(snapshot.prs), 0)
|
||||||
|
|
||||||
|
def test_queue_fail_closed_hides_empty_state_copy(self):
|
||||||
def _empty_pagination() -> PaginationMeta:
|
with mock.patch("webui.queue_loader.get_auth_header", return_value=None):
|
||||||
return PaginationMeta(
|
snapshot = load_queue_snapshot()
|
||||||
page=1,
|
|
||||||
per_page=50,
|
|
||||||
returned_count=0,
|
|
||||||
has_more=False,
|
|
||||||
is_final_page=True,
|
|
||||||
inventory_complete=True,
|
|
||||||
pages_fetched=1,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
def _empty_fetch(*_args, **_kwargs):
|
|
||||||
return [], _empty_pagination()
|
|
||||||
|
|
||||||
|
|
||||||
class TestQueueFailClosedUx(unittest.TestCase):
|
|
||||||
"""Regression tests for #458 fail-closed empty-state copy."""
|
|
||||||
|
|
||||||
def test_fail_closed_view_suppresses_empty_queue_copy(self):
|
|
||||||
snapshot = QueueSnapshot(
|
|
||||||
project_id="gitea-tools",
|
|
||||||
repo_label="Scaled-Tech-Consulting/Gitea-Tools",
|
|
||||||
prs=(),
|
|
||||||
issues=(),
|
|
||||||
pr_pagination=None,
|
|
||||||
issue_pagination=None,
|
|
||||||
fetch_error="Gitea credentials unavailable for gitea.prgs.cc",
|
|
||||||
)
|
|
||||||
html = render_queue_page(snapshot)
|
|
||||||
self.assertIn("Queue unavailable", html)
|
|
||||||
self.assertIn("Not loaded", html)
|
|
||||||
self.assertNotIn("No open items.", html)
|
|
||||||
self.assertIn("pagination:</strong> unavailable", html)
|
|
||||||
|
|
||||||
def test_fail_closed_route_does_not_show_empty_queue_copy(self):
|
|
||||||
client = TestClient(create_app())
|
|
||||||
snapshot = load_queue_snapshot(
|
|
||||||
fetch_prs=_empty_fetch,
|
|
||||||
fetch_issues=_empty_fetch,
|
|
||||||
)
|
|
||||||
snapshot = QueueSnapshot(
|
|
||||||
project_id=snapshot.project_id,
|
|
||||||
repo_label=snapshot.repo_label,
|
|
||||||
prs=(),
|
|
||||||
issues=(),
|
|
||||||
pr_pagination=None,
|
|
||||||
issue_pagination=None,
|
|
||||||
fetch_error="Gitea credentials unavailable for gitea.prgs.cc",
|
|
||||||
)
|
|
||||||
with mock.patch("webui.app.load_queue_snapshot", return_value=snapshot):
|
with mock.patch("webui.app.load_queue_snapshot", return_value=snapshot):
|
||||||
response = client.get("/queue")
|
response = self.client.get("/queue")
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 200)
|
||||||
self.assertIn("Queue unavailable", response.text)
|
self.assertIn("Queue unavailable", response.text)
|
||||||
|
self.assertIn("not fetched", response.text.lower())
|
||||||
self.assertNotIn("No open items.", response.text)
|
self.assertNotIn("No open items.", response.text)
|
||||||
|
self.assertNotIn("Open pull requests", response.text)
|
||||||
|
self.assertNotIn("pages_fetched", response.text)
|
||||||
|
|
||||||
|
def test_queue_successful_empty_inventory_shows_empty_state(self):
|
||||||
|
def _empty_prs(*_args, **_kwargs):
|
||||||
|
return [], _mock_pagination(0)
|
||||||
|
|
||||||
|
def _empty_issues(*_args, **_kwargs):
|
||||||
|
return [], _mock_pagination(0)
|
||||||
|
|
||||||
def test_successful_empty_inventory_shows_empty_copy(self):
|
|
||||||
snapshot = load_queue_snapshot(
|
snapshot = load_queue_snapshot(
|
||||||
fetch_prs=_empty_fetch,
|
fetch_prs=_empty_prs,
|
||||||
fetch_issues=_empty_fetch,
|
fetch_issues=_empty_issues,
|
||||||
)
|
)
|
||||||
|
with mock.patch("webui.app.load_queue_snapshot", return_value=snapshot):
|
||||||
|
response = self.client.get("/queue")
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
self.assertIsNone(snapshot.fetch_error)
|
self.assertIsNone(snapshot.fetch_error)
|
||||||
self.assertEqual(len(snapshot.prs), 0)
|
self.assertEqual(response.text.count("No open items."), 2)
|
||||||
self.assertEqual(len(snapshot.issues), 0)
|
self.assertIn("pages_fetched", response.text)
|
||||||
self.assertTrue(snapshot.pr_pagination.inventory_complete)
|
self.assertIn("pagination (complete)", response.text)
|
||||||
|
|
||||||
html = render_queue_page(snapshot)
|
|
||||||
self.assertNotIn("Queue unavailable", html)
|
|
||||||
self.assertEqual(html.count("No open items."), 2)
|
|
||||||
self.assertIn("pagination (complete)", html)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
@@ -1,153 +0,0 @@
|
|||||||
"""Tests for runtime_context / mutation-guard workspace alignment (#460)."""
|
|
||||||
|
|
||||||
from __future__ import annotations
|
|
||||||
|
|
||||||
import os
|
|
||||||
import sys
|
|
||||||
import unittest
|
|
||||||
from pathlib import Path
|
|
||||||
from unittest import mock
|
|
||||||
from unittest.mock import MagicMock
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
import author_mutation_worktree as amw # noqa: E402
|
|
||||||
import gitea_mcp_server as srv # noqa: E402
|
|
||||||
|
|
||||||
CONTROL_ROOT = str(Path(__file__).resolve().parents[3])
|
|
||||||
BRANCHES_WORKTREE = str(Path(__file__).resolve().parents[1])
|
|
||||||
MCP_PROCESS_ROOT = BRANCHES_WORKTREE
|
|
||||||
|
|
||||||
|
|
||||||
class TestCanonicalRepoRoot(unittest.TestCase):
|
|
||||||
@mock.patch("subprocess.run")
|
|
||||||
def test_resolves_main_repo_from_branches_worktree(self, mock_run):
|
|
||||||
mock_run.return_value = MagicMock(
|
|
||||||
returncode=0,
|
|
||||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
|
||||||
)
|
|
||||||
root = amw.resolve_canonical_repo_root(BRANCHES_WORKTREE, MCP_PROCESS_ROOT)
|
|
||||||
self.assertEqual(root, CONTROL_ROOT)
|
|
||||||
|
|
||||||
def test_falls_back_when_git_unavailable(self):
|
|
||||||
root = amw.resolve_canonical_repo_root("/missing/path", MCP_PROCESS_ROOT)
|
|
||||||
self.assertEqual(root, os.path.realpath(MCP_PROCESS_ROOT))
|
|
||||||
|
|
||||||
|
|
||||||
class TestWorkspaceRepoMembership(unittest.TestCase):
|
|
||||||
@mock.patch("os.path.isdir", return_value=True)
|
|
||||||
@mock.patch("os.path.exists", return_value=True)
|
|
||||||
@mock.patch("subprocess.run")
|
|
||||||
def test_valid_branches_worktree_accepted(self, mock_run, *_exists):
|
|
||||||
mock_run.return_value = MagicMock(
|
|
||||||
returncode=0,
|
|
||||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
|
||||||
)
|
|
||||||
result = amw.assess_workspace_repo_membership(
|
|
||||||
workspace_path=BRANCHES_WORKTREE,
|
|
||||||
canonical_repo_root=CONTROL_ROOT,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["proven"])
|
|
||||||
self.assertFalse(result["block"])
|
|
||||||
|
|
||||||
@mock.patch("os.path.isdir", return_value=True)
|
|
||||||
@mock.patch("os.path.exists", return_value=True)
|
|
||||||
@mock.patch("subprocess.run")
|
|
||||||
def test_wrong_repo_rejected(self, mock_run, *_exists):
|
|
||||||
mock_run.return_value = MagicMock(
|
|
||||||
returncode=0,
|
|
||||||
stdout="/other/repo/.git\n",
|
|
||||||
)
|
|
||||||
result = amw.assess_workspace_repo_membership(
|
|
||||||
workspace_path=BRANCHES_WORKTREE,
|
|
||||||
canonical_repo_root=CONTROL_ROOT,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertIn("does not belong", result["reasons"][0])
|
|
||||||
|
|
||||||
@mock.patch("os.path.exists", return_value=False)
|
|
||||||
def test_missing_worktree_rejected(self, *_exists):
|
|
||||||
result = amw.assess_workspace_repo_membership(
|
|
||||||
workspace_path=f"{CONTROL_ROOT}/branches/missing-worktree",
|
|
||||||
canonical_repo_root=CONTROL_ROOT,
|
|
||||||
)
|
|
||||||
self.assertTrue(result["block"])
|
|
||||||
self.assertIn("does not exist", result["reasons"][0])
|
|
||||||
|
|
||||||
|
|
||||||
class TestRuntimeContextGuardAlignment(unittest.TestCase):
|
|
||||||
def setUp(self):
|
|
||||||
srv._preflight_whoami_called = True
|
|
||||||
srv._preflight_capability_called = True
|
|
||||||
srv._preflight_resolved_role = "author"
|
|
||||||
self._orig_in_test = srv._preflight_in_test_mode
|
|
||||||
srv._preflight_in_test_mode = lambda: False
|
|
||||||
self._env_patch = mock.patch.dict(
|
|
||||||
os.environ,
|
|
||||||
{},
|
|
||||||
clear=False,
|
|
||||||
)
|
|
||||||
self._env_patch.start()
|
|
||||||
os.environ.pop("GITEA_ACTIVE_WORKTREE", None)
|
|
||||||
os.environ.pop("GITEA_AUTHOR_WORKTREE", None)
|
|
||||||
|
|
||||||
def tearDown(self):
|
|
||||||
srv._preflight_in_test_mode = self._orig_in_test
|
|
||||||
self._env_patch.stop()
|
|
||||||
|
|
||||||
def test_runtime_context_and_guard_share_resolved_workspace(self):
|
|
||||||
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
|
|
||||||
ctx = srv._resolve_author_mutation_context(BRANCHES_WORKTREE)
|
|
||||||
status = srv.assess_preflight_status(worktree_path=BRANCHES_WORKTREE)
|
|
||||||
self.assertEqual(ctx["workspace_path"], os.path.realpath(BRANCHES_WORKTREE))
|
|
||||||
self.assertEqual(ctx["canonical_repo_root"], CONTROL_ROOT)
|
|
||||||
self.assertFalse(ctx["roots_aligned"])
|
|
||||||
self.assertEqual(
|
|
||||||
status["preflight_workspace"]["active_task_workspace_root"],
|
|
||||||
os.path.realpath(BRANCHES_WORKTREE),
|
|
||||||
)
|
|
||||||
self.assertEqual(
|
|
||||||
status["preflight_workspace"]["canonical_repository_root"],
|
|
||||||
CONTROL_ROOT,
|
|
||||||
)
|
|
||||||
self.assertIn("workspace_root_mismatch", status["preflight_workspace"])
|
|
||||||
|
|
||||||
@mock.patch("subprocess.run")
|
|
||||||
@mock.patch("os.path.isdir", return_value=True)
|
|
||||||
@mock.patch("os.path.exists", return_value=True)
|
|
||||||
def test_declared_branches_worktree_passes_when_mcp_root_differs(
|
|
||||||
self, _exists, _isdir, mock_run
|
|
||||||
):
|
|
||||||
mock_run.return_value = MagicMock(
|
|
||||||
returncode=0,
|
|
||||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
|
||||||
)
|
|
||||||
with mock.patch.object(srv, "PROJECT_ROOT", MCP_PROCESS_ROOT):
|
|
||||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
|
||||||
srv.verify_preflight_purity(worktree_path=BRANCHES_WORKTREE)
|
|
||||||
|
|
||||||
def test_stable_checkout_still_rejected(self):
|
|
||||||
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
|
|
||||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
|
||||||
srv.verify_preflight_purity()
|
|
||||||
self.assertIn("stable control checkout", str(ctx.exception))
|
|
||||||
|
|
||||||
@mock.patch("os.path.isdir", return_value=True)
|
|
||||||
@mock.patch("os.path.exists", return_value=True)
|
|
||||||
@mock.patch("subprocess.run")
|
|
||||||
def test_non_branches_worktree_rejected(self, mock_run, *_exists):
|
|
||||||
outside = "/tmp/outside-repo-checkout"
|
|
||||||
mock_run.return_value = MagicMock(
|
|
||||||
returncode=0,
|
|
||||||
stdout=f"{CONTROL_ROOT}/.git\n",
|
|
||||||
)
|
|
||||||
with mock.patch.object(srv, "PROJECT_ROOT", CONTROL_ROOT):
|
|
||||||
with mock.patch.dict("os.environ", {"GITEA_TEST_PORCELAIN": ""}, clear=False):
|
|
||||||
with self.assertRaises(RuntimeError) as ctx:
|
|
||||||
srv.verify_preflight_purity(worktree_path=outside)
|
|
||||||
self.assertIn("not under", str(ctx.exception))
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
unittest.main()
|
|
||||||
+9
-26
@@ -20,50 +20,33 @@ def run(script, *args):
|
|||||||
branch = arg
|
branch = arg
|
||||||
break
|
break
|
||||||
|
|
||||||
lock_dir_ctx = None
|
lock_file = Path("/tmp/gitea_issue_lock.json")
|
||||||
extra_env = os.environ.copy()
|
created_lock = False
|
||||||
if script == "worktree-start" and branch:
|
if script == "worktree-start" and branch:
|
||||||
import re
|
import re
|
||||||
import tempfile
|
import json
|
||||||
import issue_lock_store
|
|
||||||
|
|
||||||
m = re.search(r"issue-(\d+)", branch)
|
m = re.search(r"issue-(\d+)", branch)
|
||||||
if not m:
|
if not m:
|
||||||
m = re.search(r"pr-(\d+)", branch)
|
m = re.search(r"pr-(\d+)", branch)
|
||||||
issue_num = int(m.group(1)) if m else 999
|
issue_num = int(m.group(1)) if m else 999
|
||||||
lock_dir_ctx = tempfile.TemporaryDirectory()
|
lock_file.write_text(json.dumps({
|
||||||
extra_env["GITEA_ISSUE_LOCK_DIR"] = lock_dir_ctx.name
|
|
||||||
record = {
|
|
||||||
"issue_number": issue_num,
|
"issue_number": issue_num,
|
||||||
"branch_name": branch,
|
"branch_name": branch,
|
||||||
"remote": "prgs",
|
"remote": "prgs",
|
||||||
"org": "Scaled-Tech-Consulting",
|
"org": "Scaled-Tech-Consulting",
|
||||||
"repo": "Gitea-Tools",
|
"repo": "Gitea-Tools"
|
||||||
"worktree_path": "/tmp/test-worktree",
|
}), encoding="utf-8")
|
||||||
"work_lease": {
|
created_lock = True
|
||||||
"operation_type": "author_issue_work",
|
|
||||||
"expires_at": "2999-01-01T00:00:00Z",
|
|
||||||
},
|
|
||||||
}
|
|
||||||
path = issue_lock_store.lock_file_path(
|
|
||||||
remote="prgs",
|
|
||||||
org="Scaled-Tech-Consulting",
|
|
||||||
repo="Gitea-Tools",
|
|
||||||
issue_number=issue_num,
|
|
||||||
lock_dir=lock_dir_ctx.name,
|
|
||||||
)
|
|
||||||
issue_lock_store.save_lock_file(path, record)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
proc = subprocess.run(
|
proc = subprocess.run(
|
||||||
["bash", str(SCRIPTS / script), *args],
|
["bash", str(SCRIPTS / script), *args],
|
||||||
capture_output=True, text=True, cwd=str(REPO),
|
capture_output=True, text=True, cwd=str(REPO),
|
||||||
env=extra_env,
|
|
||||||
)
|
)
|
||||||
return proc.returncode, proc.stdout, proc.stderr
|
return proc.returncode, proc.stdout, proc.stderr
|
||||||
finally:
|
finally:
|
||||||
if lock_dir_ctx is not None:
|
if created_lock and lock_file.exists():
|
||||||
lock_dir_ctx.cleanup()
|
lock_file.unlink()
|
||||||
|
|
||||||
|
|
||||||
class TestWorktreeStart(unittest.TestCase):
|
class TestWorktreeStart(unittest.TestCase):
|
||||||
|
|||||||
+12
-10
@@ -38,13 +38,7 @@ def _queue_table(
|
|||||||
title: str,
|
title: str,
|
||||||
items: tuple[QueueItem, ...],
|
items: tuple[QueueItem, ...],
|
||||||
columns: tuple[tuple[str, str], ...],
|
columns: tuple[tuple[str, str], ...],
|
||||||
fetch_failed: bool = False,
|
|
||||||
) -> str:
|
) -> str:
|
||||||
if fetch_failed:
|
|
||||||
return (
|
|
||||||
f"<h3>{html.escape(title)}</h3>"
|
|
||||||
"<p class='muted'>Not loaded — queue fetch did not complete.</p>"
|
|
||||||
)
|
|
||||||
if not items:
|
if not items:
|
||||||
return f"<h3>{html.escape(title)}</h3><p class='muted'>No open items.</p>"
|
return f"<h3>{html.escape(title)}</h3><p class='muted'>No open items.</p>"
|
||||||
|
|
||||||
@@ -78,12 +72,22 @@ def render_queue_page(snapshot: QueueSnapshot) -> str:
|
|||||||
f'<div class="stub"><p><strong>Queue unavailable:</strong> '
|
f'<div class="stub"><p><strong>Queue unavailable:</strong> '
|
||||||
f"{html.escape(snapshot.fetch_error)}</p></div>"
|
f"{html.escape(snapshot.fetch_error)}</p></div>"
|
||||||
)
|
)
|
||||||
|
inventory_block = (
|
||||||
|
"<p class='muted'><strong>Inventory:</strong> not fetched "
|
||||||
|
"(fail closed — queue tables omitted).</p>"
|
||||||
|
)
|
||||||
|
return (
|
||||||
|
"<h2>Live queue</h2>"
|
||||||
|
f"<p class='meta'>Repository: <code>{html.escape(snapshot.repo_label)}</code> "
|
||||||
|
f"· project <code>{html.escape(snapshot.project_id)}</code></p>"
|
||||||
|
f"{error_block}"
|
||||||
|
f"{inventory_block}"
|
||||||
|
"<p class='muted'>Read-only MVP — claims, reviews, and merges stay in Gitea/MCP tools.</p>"
|
||||||
|
)
|
||||||
|
|
||||||
fetch_failed = bool(snapshot.fetch_error)
|
|
||||||
pr_section = _queue_table(
|
pr_section = _queue_table(
|
||||||
title="Open pull requests",
|
title="Open pull requests",
|
||||||
items=snapshot.prs,
|
items=snapshot.prs,
|
||||||
fetch_failed=fetch_failed,
|
|
||||||
columns=(
|
columns=(
|
||||||
("number", "#"),
|
("number", "#"),
|
||||||
("title", "Title"),
|
("title", "Title"),
|
||||||
@@ -96,7 +100,6 @@ def render_queue_page(snapshot: QueueSnapshot) -> str:
|
|||||||
issue_section = _queue_table(
|
issue_section = _queue_table(
|
||||||
title="Open issues",
|
title="Open issues",
|
||||||
items=snapshot.issues,
|
items=snapshot.issues,
|
||||||
fetch_failed=fetch_failed,
|
|
||||||
columns=(
|
columns=(
|
||||||
("number", "#"),
|
("number", "#"),
|
||||||
("title", "Title"),
|
("title", "Title"),
|
||||||
@@ -110,7 +113,6 @@ def render_queue_page(snapshot: QueueSnapshot) -> str:
|
|||||||
"<h2>Live queue</h2>"
|
"<h2>Live queue</h2>"
|
||||||
f"<p class='meta'>Repository: <code>{html.escape(snapshot.repo_label)}</code> "
|
f"<p class='meta'>Repository: <code>{html.escape(snapshot.repo_label)}</code> "
|
||||||
f"· project <code>{html.escape(snapshot.project_id)}</code></p>"
|
f"· project <code>{html.escape(snapshot.project_id)}</code></p>"
|
||||||
f"{error_block}"
|
|
||||||
f"{_pagination_html('PR', snapshot.pr_pagination)}"
|
f"{_pagination_html('PR', snapshot.pr_pagination)}"
|
||||||
f"{pr_section}"
|
f"{pr_section}"
|
||||||
f"{_pagination_html('Issue', snapshot.issue_pagination)}"
|
f"{_pagination_html('Issue', snapshot.issue_pagination)}"
|
||||||
|
|||||||
Reference in New Issue
Block a user