Compare commits

...
Author SHA1 Message Date
sysadmin db5d14184f feat: controller-owned allocator API on control-plane DB (Closes #600)
Add gitea_allocate_next_work and allocator_service routing policy on top of
the #613 ControlPlaneDB substrate. Workers get atomic assign+lease results
(or wait/no_safe_work/terminal-path outcomes) without self-selecting work
via file locks or comment-only leases. #612 remains downstream.

Closes #600
2026-07-10 02:54:20 -04:00
sysadmin 10228e1c06 Merge pull request 'feat: control-plane DB substrate for atomic assign/lease (Closes #613)' (#619) from feat/issue-613-allocator-db-substrate into master 2026-07-10 01:44:19 -05:00
sysadmin d8b2b8f1a7 Merge pull request 'fix: head-scope durable #332 review-decision locks (Closes #620)' (#621) from fix/issue-620-head-scoped-review-locks into master
Reviewed-on: #621
2026-07-10 01:12:57 -05:00
sysadmin 9e1d5c5649 fix: head-scope durable #332 review-decision locks (#620)
Prior REQUEST_CHANGES on head A no longer blocks formal re-review on
head B of the same open PR. Locks store reviewed head SHA, hard-stop and
mark_ready/submit gates allow a fresh decision boundary when the live
head differs, and same-head duplicates remain fail-closed. Open-PR lock
cleanup stays forbidden (#594); assessment reports locked vs current
head and stale_by_head / fresh_review_on_current_head_allowed.

Closes #620
2026-07-10 01:04:37 -04:00
sysadmin 2429d9d2e8 fix: fail closed on conflicting incident_links observation metadata
Legacy NULL-scope dedupe only compared Gitea targets, so duplicate rows
with the same provider key and issue but different fingerprint/status/
event_count/etc. collapsed to the lowest link_id and silently dropped
observation data. Migration now compares all meaningful observation
fields and refuses to discard conflicts (#619 RC3 / #613).
2026-07-09 23:04:17 -04:00
sysadmin 16cd871fbd fix: safe incident_links migration order; require PR head pin
Address remaining PR #619 blockers on head 783ea88:

- Deduplicate legacy NULL-scope incident_links before normalizing to ''
- Fail closed when duplicate rows disagree on Gitea target
- Require non-empty expected_head_sha for PR assign and mutation
2026-07-09 22:51:27 -04:00
sysadmin 783ea88a01 fix: fail closed on stale/terminal assignments; canonicalize incident_links
Address REQUEST_CHANGES on PR #619:

- require_valid_assignment rejects terminal work states and head drift
- incident_links scope keys normalize NULL/blank to '' for UNIQUE
- remove trailing whitespace in control-plane-db-substrate.md
2026-07-09 22:41:25 -04:00
sysadmin 036b78e31e feat: control-plane DB substrate for atomic assign/lease (Closes #613)
Add SQLite single-writer MVP control plane per the allocator ADR:
sessions, work_items (issue/pr only), atomic assign+lease, mutation
gates, terminal-lock index, and provider-neutral incident_links.

DB coordinates concurrency; Gitea remains assignable work; raw
Sentry/GlitchTip incidents are never work items. #600 and #612 build on
this substrate.
2026-07-09 22:27:51 -04:00
sysadmin 08c3488d7c Merge pull request 'docs: ADR for MCP allocator, control-plane DB, and incident bridge (#613)' (#614) from docs/issue-613-allocator-control-plane-observability-adr into master 2026-07-09 21:10:27 -05:00
sysadmin 4f2fd9a8b1 fix(mcp): resolve default remote mapping and connection check bugs 2026-07-09 20:26:58 -04:00
sysadmin 4185ee1417 docs: ADR for MCP allocator, control-plane DB, and incident bridge
Canonical architecture for #613#600#612: DB coordinates, Gitea
records, Sentry/GlitchTip observe; allocator assigns only Gitea issues/PRs.

Refs: #600 #612 #613
2026-07-09 20:00:59 -04:00
sysadmin ae6a3ae00c Merge pull request 'feat: diagnose live MCP namespace EOF health and block merge (#543)' (#587) from feat/issue-543-mcp-namespace-health-check into master 2026-07-09 18:46:52 -05:00
sysadmin 2fde92ad25 Implement review drafts saving and resuming (#609) 2026-07-09 19:08:28 -04:00
sysadmin ebd06b73c9 fix: address PR #587 REQUEST_CHANGES for MCP namespace health (#543)
- Distinguish client_namespace vs offline_spawn probe sources; only IDE
  client probes prove namespace health and feed mutation gates.
- Record assessments in session; gate gitea_submit_pr_review and
  gitea_merge_pr when client-namespace health is unhealthy/non-proven.
- Mark test_mcp_conn.py offline-only; align recovery docs to client
  reconnect (no PID-kill/config-touch as canonical recovery).
- Rebase onto current master so #590 ledger isolation keeps
  TestMergePR.test_unknown_profile_blocks green.
2026-07-09 18:44:04 -04:00
sysadmin af9f1c5944 style: fix trailing newline in mcp_namespace_health.py 2026-07-09 18:39:41 -04:00
sysadminandClaude Opus 4.8 f83d2c2027 docs: add MCP namespace client is closing: EOF recovery runbook (#543)
Adds docs/mcp-namespace-eof-recovery.md documenting the correct recovery
path when a Gitea MCP namespace (gitea-author/reviewer/merger/tools)
returns `client is closing: EOF`.

Satisfies acceptance criterion 7 of #543: symptom, root cause (closed
client transport vs a live-but-stale process), the sanctioned
reconnect/relaunch sequence, diagnostics to capture, and how
test_mcp_conn.py reproduces the registered-vs-callable gap. Distinguishes
this transport-close failure from the ps-based stale-runtime family in
#531/#544 and reinforces the no-direct-import guard (#558).

Docs-only; no code or test behavior changed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-09 18:39:41 -04:00
sysadminandClaude Opus 4.8 93781af7e9 feat: enforce live-namespace block in review/merge state machine (#543 AC5)
The namespace health check (05fdcee) classified a false-ready namespace but
only returned an advisory blocks_merge_workflow flag; nothing in the merge
gate consumed it. Wire it in so a broken live namespace hard-blocks merge.

- review_merge_state_machine.assess_workflow_blockers: add live_namespace_broken
  blocker (registered-in-FastMCP but not callable-through-namespace).
- assess_state_advancement: forward **blocker_kwargs so can_approve/can_merge/
  workflow_status honor the full blocker set (also fixes latent drop of
  mcp_reconnect_failed/stale_capability_state through those paths).
- gitea_assess_review_merge_state_machine tool: accept live_namespace_broken and
  thread it through all state-machine calls.
- Tests: prove can_merge/workflow_status/tool block on live_namespace_broken even
  with every review state + pre-merge gate satisfied; bridge classify verdict.
- Docs: enforcement section wiring blocks_merge_workflow -> live_namespace_broken.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-09 18:39:41 -04:00
sysadmin 314615ec2c feat: diagnose live MCP namespace EOF health 2026-07-09 18:39:41 -04:00
sysadmin 52013791d4 Merge pull request 'feat: canonical validation wording, baseline proof, and closure gate (Closes #529)' (#598) from feat/issue-529-canonical-validation-wording into master 2026-07-09 17:30:52 -05:00
sysadmin 2ac7519792 Merge pull request 'fix: enforce merger role boundaries (Closes #539)' (#596) from fix/issue-539-role-boundary-hard-stops into master 2026-07-09 17:07:54 -05:00
sysadmin a32c68e24b Merge pull request 'feat: diagnose reviewer lease handoff next actions (Closes #599)' (#608) from feat/issue-599-reviewer-lease-handoff into master 2026-07-09 16:47:23 -05:00
sysadmin 7186718cfd Merge pull request 'fix: isolate review-mutation ledger from host session-state (Closes #590)' (#592) from fix/issue-590-ledger-isolation into master 2026-07-09 16:32:02 -05:00
sysadmin 964505654f feat: diagnose reviewer lease handoff next actions (#599)
Add read-only diagnose_reviewer_pr_lease_handoff and MCP tool
gitea_diagnose_reviewer_pr_lease_handoff so open-PR foreign leases,
instructed-lease replacement, reclaimable release, and worktree
binding mismatch return a canonical next_action without steal paths.

Closes #599
2026-07-09 17:31:14 -04:00
sysadmin 008cd3fd74 fix: keep tests free of host MCP stale-runtime probes (#590)
patch.dict(os.environ, clear=True) also drops PYTEST_CURRENT_TEST, which
re-enables live MCP process health checks against operator daemons and
makes merge-gate unit tests environment-dependent. Stub the diagnostic
in conftest so unit tests stay isolated.
2026-07-09 17:20:09 -04:00
sysadmin 11ffe0f9dd fix: isolate review-mutation ledger from host session-state (#590)
Tests that call patch.dict(os.environ, clear=True) dropped
GITEA_MCP_SESSION_STATE_DIR and re-adopted the operator host cache
under ~/.cache/gitea-tools/session-state. A residual terminal
request_changes mutation then tripped the #332 hard-stop before
test_unknown_profile_blocks could assert the empty-profile gate.

- Pin mcp_session_state.default_state_dir / DEFAULT_STATE_DIR to a
  per-test temp dir in conftest (still honors an explicit env override)
- Clear the decision lock at the start of each TestMergePR test
- Add regression coverage for env-clear isolation

Closes #590
2026-07-09 17:19:29 -04:00
sysadmin 683649424b Merge pull request 'feat: auto-restart Gitea MCP namespaces when stale or git HEAD advances (#591)' (#593) from feat/issue-591-auto-restart-mcp into master 2026-07-09 15:46:34 -05:00
sysadmin ac531dda05 Merge pull request 'feat: canonical cleanup for stale #332 review decision locks (Closes #594)' (#595) from feat/issue-594-stale-decision-lock-cleanup into master 2026-07-09 15:01:27 -05:00
sysadminandClaude Opus 4.8 36781ebef7 feat: post-merge moot validation wording + validation:* labels (#529)
Adds #529 acceptance criteria 1/4/5.

- post_merge_validation.py: assess_post_merge_validation defines the four
  canonical validation distinctions (clean pass / baseline-accepted / blocked /
  post-merge moot) and fails closed on two mistakes: (a) an active approval on a
  PR already merged/closed before review submission, and (b) post-merge moot
  validation claimed without merged-state + merge-commit proof.
- issue_workflow_labels.py: durable validation:* process-state labels
  (clean-pass / baseline-accepted / blocked / post-merge-moot) registered in
  CANONICAL_LABEL_SPECS so manage_labels creates them.
- final_report_validator.py: _rule_reviewer_post_merge_validation wired into
  the review_pr rule set.
- tests/test_post_merge_validation.py: module behavior, label registration,
  and the wired review rule.

Refs #529

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-09 15:49:14 -04:00
sysadmin 2941ec529c fix: enforce merger role boundaries (Closes #539) 2026-07-09 15:41:07 -04:00
sysadminandClaude Opus 4.8 eddb68818e feat: block issue closure on unproven expected pre-existing failure (#529)
Delivers #529 acceptance criterion 6: controller issue closure must be
blocked when the closure report calls a non-zero test-suite exit an
"expected pre-existing"/baseline failure without pre-merge proof.

- controller_closure_baseline_proof.py: assess_controller_closure_baseline_proof
  reuses the #533 pre-merge baseline verifier; empty report -> skipped/allowed,
  clean pass -> allowed, proven baseline -> allowed, unproven baseline -> block.
- gitea_close_issue: optional closure_report param; fails closed (no state PATCH)
  when the gate blocks.
- final_report_validator: new controller_close task kind (alias close_issue)
  wired to the pre-merge baseline proof rule so a closure can be pre-checked.
- Tests: tests/test_controller_closure_baseline_proof.py plus two
  gitea_close_issue gate tests.

Scope: criteria 2/3 (non-zero exit not a clean pass; baseline claims need
proof) are already enforced on master via premerge_baseline_proof (#533) and
the reviewer schema rules. Criteria 1/4/5 (post-merge moot canonical wording
and validation:* process-state labels) remain follow-up work.

Validation: full suite 2365 passed, 6 skipped; the 9 remaining failures
(tests/test_config.py TestAuthIntegration, tests/test_credentials.py
TestGetCredentials, test_issue_540 reviewer-role) are baseline-proven —
identical failures on clean prgs/master 6913ac9 (keychain/env dependent),
unrelated to this change.

Refs #529

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-09 15:39:54 -04:00
sysadmin b98e8dfa1a test: harden session-state isolation for #594 decision-lock suite
Pin per-test durable session-state so patch.dict(clear=True) cannot adopt
host ~/.cache review-decision locks, and clear the merge-test ledger in
setUp. Keeps active #332 hard-stop coverage while making TestMergePR
deterministic without manual host cache cleanup.

Aligned with #590 / PR #592 isolation approach.
2026-07-09 15:24:41 -04:00
sysadmin 86651a3d05 docs: expand #594 stale decision-lock cleanup guidance
Add allowed/forbidden workflow steps to the review-merge skill and
document the #594 cleanup gate on the Safety-and-Gates wiki page.
2026-07-09 15:21:21 -04:00
sysadmin 3f3f880147 feat: canonical cleanup for stale #332 review decision locks (#594)
Durable review-decision locks (#559) can outlive the PR they protect.
When the last terminal mutation references a PR that is already
merged/closed, expose gitea_cleanup_stale_review_decision_lock so a
reviewer can clear the lock with identity/profile gates and a durable
audit trail — without weakening #332 for open PRs or deleting
session-state files by hand.

Also auto-clear same-profile locks after a successful merge of the
approved PR, and document allowed vs forbidden cleanup.

Closes #594
2026-07-09 15:18:48 -04:00
sysadmin 6094069ef6 feat: auto-restart Gitea MCP namespaces when stale or git HEAD advances (closes #591) 2026-07-09 14:20:05 -04:00
sysadmin 6913ac98f1 Merge pull request 'feat: block vague-reference issue creation with content preflight (Closes #582)' (#586) from feat/issue-582-issue-content-preflight into master 2026-07-09 13:20:02 -05:00
sysadmin faf36b5bdf Merge pull request 'feat: merge-report lease proof fields and handoff audit (Closes #535)' (#585) from feat/issue-535-lease-proof-report into master 2026-07-09 13:16:19 -05:00
sysadmin 05b4f13e96 Merge pull request 'fix: control-plane guide org/repo params after #530 guard (Closes #588)' (#589) from fix/issue-588-guide-repo-guard into master 2026-07-09 12:49:24 -05:00
sysadmin 57b2023611 fix: control-plane guide org/repo params and local repo alignment (#588)
#530's remote/repo guard blocked mcp_get_control_plane_guide(remote=prgs)
because the bare default resolves to Timesheet while local git is
Gitea-Tools, and remediation told callers to pass org/repo on a tool that
did not accept those parameters.

- Accept optional org/repo on mcp_get_control_plane_guide
- Prefer local git remote org/repo when bare defaults mismatch (read-only guide)
- Keep mutation tools fail-closed on bare mismatch
- Align remediation text with tools that accept org/repo

Closes #588
2026-07-09 13:17:27 -04:00
sysadminandClaude Opus 4.8 5debfcf178 feat: block vague-reference issue creation with content preflight (Closes #582)
Add a pre-create content gate to gitea_create_issue that fails closed with
BLOCKED + DIAGNOSE when the title/body is a vague reference to out-of-band
draft content ("the drafted issue", "the prepared issue", "the issue we
discussed", "the previous draft") and no durable source pointer (existing
issue/PR/comment, checked-in file, URL, or scratchpad path) is present.

- issue_content_gate.py: assess_issue_content / pre_create_issue_content_gate
  with vague-phrase detection, durable-source-pointer escape hatch, and a
  domination check so long specific bodies are not falsely blocked.
- gitea_create_issue: runs the gate after preflight purity, before duplicate
  search; new allow_incomplete_content override (off by default). Title-only
  creation stays allowed (no empty-body requirement) to preserve behavior.
- tests/test_issue_content_gate.py: unit + MCP integration coverage.
- create-issue.md: documents the enforced preflight with valid/invalid
  prompt examples.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-09 13:07:53 -04:00
sysadmin 27d75facd4 Merge pull request 'feat: reconciler supersession close path for PRs/issues (Closes #525)' (#583) from feat/issue-525-reconciler-supersession-close into master 2026-07-09 11:58:06 -05:00
sysadmin 2b4c291f12 feat: merge-report lease proof fields and handoff audit (#535)
Surface lease_proof_source / recovery reason from sanctioned session
provenance on gitea_merge_pr results, and block final reports that claim
manual/seeded/injected lease proof without MCP adoption evidence.

Closes #535
2026-07-09 12:56:12 -04:00
sysadmin b0ae1605c3 feat: add reconciler supersession close gate 2026-07-09 12:36:50 -04:00
49 changed files with 10174 additions and 109 deletions
+610
View File
@@ -0,0 +1,610 @@
"""Controller-owned work allocator policy (#600).
Builds on the #613 control-plane DB substrate (``ControlPlaneDB.assign_and_lease``).
Workers must not self-select exclusive work under the standard multi-LLM
workflow. They call ``gitea_allocate_next_work`` which:
1. Inspects candidate Gitea issues/PRs (never raw monitoring incidents).
2. Applies ADR routing policy (role, terminal path, leases, blocked, deps).
3. Atomically assigns + leases the selected item in one DB transaction.
This module is pure selection + substrate orchestration. Gitea I/O for live
inventory lives in the MCP tool wrapper so tests can inject candidates.
#612 remains downstream: bridge-created Gitea issues become candidates only
after they exist as normal issues; this module never assigns incidents.
"""
from __future__ import annotations
import os
import uuid
from dataclasses import dataclass, field
from typing import Any, Sequence
from control_plane_db import (
ControlPlaneDB,
ControlPlaneError,
InvalidWorkKindError,
LeaseRequiredError,
WORK_KINDS,
)
# Outcomes required by #600 / ADR §5.
OUTCOME_ASSIGNED = "assigned_work"
OUTCOME_WAIT = "wait"
OUTCOME_BLOCKED_TERMINAL = "blocked_by_terminal_path"
OUTCOME_BLOCKED_LEASE = "blocked_by_active_lease"
OUTCOME_NEEDS_CONTROLLER = "needs_controller"
OUTCOME_NO_SAFE = "no_safe_work"
OUTCOME_ROLE_INELIGIBLE = "role_ineligible"
OUTCOME_PREVIEW = "preview" # dry-run only (apply=false)
ROLE_AUTHOR = "author"
ROLE_REVIEWER = "reviewer"
ROLE_MERGER = "merger"
ROLE_RECONCILER = "reconciler"
ROLE_CONTROLLER = "controller"
VALID_ROLES = frozenset(
{ROLE_AUTHOR, ROLE_REVIEWER, ROLE_MERGER, ROLE_RECONCILER, ROLE_CONTROLLER}
)
# Default action matrices by role (mutation gate will re-check).
ROLE_ACTIONS: dict[str, tuple[tuple[str, ...], tuple[str, ...]]] = {
ROLE_AUTHOR: (
("implement", "comment", "push", "create_pr"),
("approve", "merge", "request_changes", "self_select_without_assignment"),
),
ROLE_REVIEWER: (
("review", "comment", "approve", "request_changes"),
("merge", "push", "create_pr", "self_select_without_assignment"),
),
ROLE_MERGER: (
("merge", "comment"),
("approve", "request_changes", "push", "create_pr", "self_select_without_assignment"),
),
ROLE_RECONCILER: (
("comment", "diagnose", "cleanup"),
("approve", "merge", "push", "create_pr", "self_select_without_assignment"),
),
ROLE_CONTROLLER: (
("comment", "diagnose", "allocate"),
("approve", "merge", "push", "create_pr"),
),
}
@dataclass
class WorkCandidate:
"""One assignable Gitea issue or PR presented to the allocator."""
kind: str # issue | pr
number: int
state: str = "open"
labels: tuple[str, ...] = ()
title: str = ""
priority: int = 0
head_sha: str | None = None
# Routing signals (callers derive from Gitea / review feedback).
request_changes_current_head: bool = False
approval_on_current_head: bool = False
approval_stale: bool = False
approval_contaminated: bool = False
mergeable: bool = False
blocked: bool = False
dependency_unmet: bool = False
dependency_reason: str | None = None
already_claimed_elsewhere: bool = False
def __post_init__(self) -> None:
self.kind = (self.kind or "").strip().lower()
self.state = (self.state or "open").strip().lower()
self.labels = tuple(
str(x).strip().lower() for x in (self.labels or ()) if str(x).strip()
)
if self.kind not in WORK_KINDS:
raise InvalidWorkKindError(
f"candidate kind '{self.kind}' is not assignable; only "
f"{sorted(WORK_KINDS)} (never raw incidents)"
)
def as_dict(self) -> dict[str, Any]:
return {
"kind": self.kind,
"number": self.number,
"state": self.state,
"labels": list(self.labels),
"title": self.title,
"priority": self.priority,
"head_sha": self.head_sha,
"request_changes_current_head": self.request_changes_current_head,
"approval_on_current_head": self.approval_on_current_head,
"approval_stale": self.approval_stale,
"approval_contaminated": self.approval_contaminated,
"mergeable": self.mergeable,
"blocked": self.blocked,
"dependency_unmet": self.dependency_unmet,
"dependency_reason": self.dependency_reason,
}
@dataclass
class SkipRecord:
kind: str
number: int
reason: str
def as_dict(self) -> dict[str, Any]:
return {"kind": self.kind, "number": self.number, "reason": self.reason}
def normalize_role(role: str | None, *, profile_name: str | None = None) -> str:
"""Map profile/role strings to a canonical allocator role."""
raw = (role or "").strip().lower()
if raw in VALID_ROLES:
return raw
prof = (profile_name or "").strip().lower()
for token in VALID_ROLES:
if token in prof or prof.endswith(f"-{token}"):
return token
if "author" in raw:
return ROLE_AUTHOR
if "review" in raw:
return ROLE_REVIEWER
if "merg" in raw:
return ROLE_MERGER
if "reconcil" in raw:
return ROLE_RECONCILER
if "control" in raw:
return ROLE_CONTROLLER
raise ControlPlaneError(
f"unknown allocator role '{role}' (profile={profile_name!r}); "
f"expected one of {sorted(VALID_ROLES)}"
)
def expected_role_for_candidate(c: WorkCandidate) -> str:
"""ADR §5.3 routing: which role should take this work next."""
if c.kind == "pr":
if c.approval_contaminated:
return ROLE_RECONCILER
if c.request_changes_current_head:
return ROLE_AUTHOR
if c.approval_stale:
return ROLE_REVIEWER
if c.approval_on_current_head and c.mergeable:
return ROLE_MERGER
# Open PR without terminal verdict → reviewer
return ROLE_REVIEWER
# Issues: ready work → author by default; blocked stays controller/none
labels = set(c.labels)
if "status:blocked" in labels or c.blocked:
return ROLE_CONTROLLER
return ROLE_AUTHOR
def role_actions(role: str) -> tuple[tuple[str, ...], tuple[str, ...]]:
return ROLE_ACTIONS.get(role, ROLE_ACTIONS[ROLE_AUTHOR])
def classify_skip(
c: WorkCandidate,
*,
role: str,
terminal_pr: int | None,
) -> str | None:
"""Return skip reason, or None if candidate is selectable for *role*."""
if c.state in ("merged", "closed"):
return f"{c.kind}#{c.number} is {c.state}; never assign"
if c.blocked or "status:blocked" in c.labels:
return f"{c.kind}#{c.number} is blocked"
if c.dependency_unmet:
return (
c.dependency_reason
or f"{c.kind}#{c.number} has unmet dependencies"
)
if c.already_claimed_elsewhere:
return f"{c.kind}#{c.number} already claimed elsewhere"
if c.kind == "pr" and not (c.head_sha or "").strip():
return f"pr#{c.number} missing head_sha pin"
# Terminal path first: when an active terminal PR exists, only that PR
# (or controller diagnosis) is assignable for review-path roles.
if terminal_pr is not None and c.kind == "pr" and c.number != terminal_pr:
if role in (ROLE_REVIEWER, ROLE_MERGER):
return (
f"pr#{c.number} skipped: active terminal-review lock on "
f"PR #{terminal_pr} must be resolved first"
)
expected = expected_role_for_candidate(c)
if role == ROLE_CONTROLLER:
# Controller may inspect anything but only assigns diagnosis targets
# when contaminated / blocked.
if expected == ROLE_RECONCILER or c.blocked:
return None
return f"{c.kind}#{c.number} does not require controller (expected {expected})"
if role != expected:
return (
f"{c.kind}#{c.number} expects role '{expected}', active role is '{role}'"
)
# Ready-gate for issues: prefer status:ready when labels present.
if c.kind == "issue" and c.labels:
if "status:ready" not in c.labels and "status:in-progress" not in c.labels:
# Allow unlabeled open issues; only skip explicit non-ready states.
if any(l.startswith("status:") for l in c.labels):
return f"issue#{c.number} not status:ready ({','.join(c.labels)})"
return None
def sort_candidates(candidates: Sequence[WorkCandidate]) -> list[WorkCandidate]:
"""Higher priority first; then lower number (older issues) for stability."""
return sorted(
candidates,
key=lambda c: (-int(c.priority), c.kind != "pr", int(c.number)),
)
def allocate_next_work(
db: ControlPlaneDB,
*,
session_id: str,
role: str,
remote: str,
org: str,
repo: str,
candidates: Sequence[WorkCandidate],
apply: bool = False,
profile_name: str | None = None,
username: str | None = None,
lease_ttl_seconds: int | None = None,
) -> dict[str, Any]:
"""Select and optionally reserve the next work unit via control-plane DB.
*apply=False* (default): dry-run selection only — no lease/assignment.
*apply=True*: atomic ``assign_and_lease`` for the selected candidate.
Never uses file locks or comment-only leases as the assignment source.
"""
if db is None:
return {
"success": False,
"outcome": OUTCOME_NO_SAFE,
"reasons": [
"control-plane DB substrate unavailable (fail closed, #600/#613)"
],
"skipped": [],
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
try:
role_norm = normalize_role(role, profile_name=profile_name)
except ControlPlaneError as exc:
return {
"success": False,
"outcome": OUTCOME_ROLE_INELIGIBLE,
"reasons": [str(exc)],
"skipped": [],
"assignment": None,
"substrate": "control_plane_db",
}
session_id = (session_id or "").strip() or f"alloc-{uuid.uuid4().hex[:12]}"
try:
db.upsert_session(
session_id=session_id,
role=role_norm,
profile=profile_name,
pid=os.getpid(),
)
except Exception as exc: # noqa: BLE001 — surface structured
return {
"success": False,
"outcome": OUTCOME_NO_SAFE,
"reasons": [
f"failed to register session in control-plane DB: {exc} "
"(fail closed, #613)"
],
"skipped": [],
"assignment": None,
"substrate": "control_plane_db",
}
# Expire stale leases globally before selection.
try:
db.expire_stale_leases()
except Exception as exc: # noqa: BLE001
return {
"success": False,
"outcome": OUTCOME_NO_SAFE,
"reasons": [f"lease expiry failed: {exc} (fail closed)"],
"skipped": [],
"assignment": None,
"substrate": "control_plane_db",
}
terminal = None
try:
terminal = db.get_active_terminal_lock(remote=remote, org=org, repo=repo)
except Exception as exc: # noqa: BLE001
return {
"success": False,
"outcome": OUTCOME_NO_SAFE,
"reasons": [f"terminal lock lookup failed: {exc} (fail closed)"],
"skipped": [],
"assignment": None,
"substrate": "control_plane_db",
}
terminal_pr = int(terminal["terminal_pr"]) if terminal else None
skipped: list[SkipRecord] = []
ordered = sort_candidates(list(candidates))
selected: WorkCandidate | None = None
for c in ordered:
reason = classify_skip(c, role=role_norm, terminal_pr=terminal_pr)
if reason:
skipped.append(SkipRecord(c.kind, c.number, reason))
continue
selected = c
break
if selected is None:
# If terminal lock blocks all review work, surface that explicitly.
if terminal_pr is not None and role_norm in (ROLE_REVIEWER, ROLE_MERGER):
outcome = OUTCOME_BLOCKED_TERMINAL
reasons = [
f"no safe work for role '{role_norm}': active terminal-review "
f"lock on PR #{terminal_pr} (resolve terminal path first, #332/#600)"
]
else:
outcome = OUTCOME_NO_SAFE
reasons = [
f"no safe assignable work for role '{role_norm}' "
f"among {len(ordered)} candidates"
]
return {
"success": True,
"outcome": outcome,
"apply": bool(apply),
"role": role_norm,
"profile_name": profile_name,
"username": username,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": None,
"expected_role_next": None,
"reasons": reasons,
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
"downstream_note": (
"#612 incident bridge remains downstream of #600; "
"allocator never assigns raw monitoring incidents"
),
}
expected_role = expected_role_for_candidate(selected)
allowed, forbidden = role_actions(role_norm)
selection = {
"kind": selected.kind,
"number": selected.number,
"title": selected.title,
"labels": list(selected.labels),
"head_sha": selected.head_sha,
"priority": selected.priority,
"expected_role_next": expected_role,
"reason_selected": (
f"highest-priority candidate for role '{role_norm}' "
f"(expected_role={expected_role})"
),
}
if not apply:
return {
"success": True,
"outcome": OUTCOME_PREVIEW,
"apply": False,
"role": role_norm,
"profile_name": profile_name,
"username": username,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": selection,
"expected_role_next": expected_role,
"reasons": [
"dry-run only (apply=false); no assignment/lease created — "
"call again with apply=true to reserve via control-plane DB"
],
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
"downstream_note": (
"#612 incident bridge remains downstream of #600; "
"allocator never assigns raw monitoring incidents"
),
}
# Atomic reserve via #613 substrate.
ttl = lease_ttl_seconds if lease_ttl_seconds is not None else None
try:
kwargs: dict[str, Any] = {
"session_id": session_id,
"role": role_norm,
"remote": remote,
"org": org,
"repo": repo,
"kind": selected.kind,
"number": selected.number,
"expected_head_sha": selected.head_sha,
"allowed_actions": allowed,
"forbidden_actions": forbidden,
"phase": "allocated",
}
if ttl is not None:
kwargs["lease_ttl_seconds"] = int(ttl)
result = db.assign_and_lease(**kwargs)
except (InvalidWorkKindError, LeaseRequiredError, ControlPlaneError) as exc:
return {
"success": False,
"outcome": OUTCOME_NO_SAFE,
"apply": True,
"role": role_norm,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": selection,
"expected_role_next": expected_role,
"reasons": [f"atomic assign+lease failed: {exc} (fail closed, #613)"],
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
if result.outcome == "wait":
return {
"success": True,
"outcome": OUTCOME_WAIT,
"apply": True,
"role": role_norm,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": selection,
"expected_role_next": expected_role,
"reasons": [result.reason or "foreign active lease"],
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": result.as_dict(),
"owner_session_id": result.owner_session_id,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
if result.outcome == "no_safe_work":
return {
"success": True,
"outcome": OUTCOME_NO_SAFE,
"apply": True,
"role": role_norm,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": selection,
"expected_role_next": expected_role,
"reasons": [result.reason or "no_safe_work"],
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": result.as_dict(),
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
# assigned
return {
"success": True,
"outcome": OUTCOME_ASSIGNED,
"apply": True,
"role": role_norm,
"profile_name": profile_name,
"username": username,
"session_id": session_id,
"remote": remote,
"org": org,
"repo": repo,
"selected": selection,
"expected_role_next": expected_role,
"reasons": [
selection["reason_selected"],
result.reason or "atomic assign+lease created",
],
"skipped": [s.as_dict() for s in skipped],
"terminal_pr": terminal_pr,
"assignment": result.as_dict(),
"lease_proof": {
"assignment_id": result.assignment_id,
"lease_id": result.lease_id,
"expires_at": result.expires_at,
"expected_head_sha": result.expected_head_sha,
"allowed_actions": list(result.allowed_actions),
"forbidden_actions": list(result.forbidden_actions),
"source": "control_plane_db.assign_and_lease",
},
"next_valid_command": _next_command(role_norm, selected),
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
"downstream_note": (
"#612 incident bridge remains downstream of #600; "
"allocator never assigns raw monitoring incidents"
),
}
def _next_command(role: str, c: WorkCandidate) -> str:
if role == ROLE_AUTHOR and c.kind == "issue":
return f"implement issue #{c.number} under a branches/ worktree; open PR when ready"
if role == ROLE_AUTHOR and c.kind == "pr":
return (
f"address REQUEST_CHANGES on PR #{c.number} at head "
f"{(c.head_sha or '')[:12]} and push fixes"
)
if role == ROLE_REVIEWER:
return (
f"review PR #{c.number} pinned at head {(c.head_sha or '')[:12]} "
"via full reviewer workflow"
)
if role == ROLE_MERGER:
return (
f"merge PR #{c.number} only with explicit operator MERGE "
f"authorization at head {(c.head_sha or '')[:12]}"
)
if role == ROLE_RECONCILER:
return f"diagnose contested state for {c.kind}#{c.number}"
return f"proceed on {c.kind}#{c.number} under role {role}"
def candidate_from_dict(data: dict[str, Any]) -> WorkCandidate:
"""Build a WorkCandidate from a plain dict (tests / MCP inventory)."""
return WorkCandidate(
kind=str(data.get("kind") or "issue"),
number=int(data["number"]),
state=str(data.get("state") or "open"),
labels=tuple(data.get("labels") or ()),
title=str(data.get("title") or ""),
priority=int(data.get("priority") or 0),
head_sha=data.get("head_sha"),
request_changes_current_head=bool(data.get("request_changes_current_head")),
approval_on_current_head=bool(data.get("approval_on_current_head")),
approval_stale=bool(data.get("approval_stale")),
approval_contaminated=bool(data.get("approval_contaminated")),
mergeable=bool(data.get("mergeable")),
blocked=bool(data.get("blocked")),
dependency_unmet=bool(data.get("dependency_unmet")),
dependency_reason=data.get("dependency_reason"),
already_claimed_elsewhere=bool(data.get("already_claimed_elsewhere")),
)
+1151
View File
File diff suppressed because it is too large Load Diff
+63
View File
@@ -0,0 +1,63 @@
"""Controller-closure baseline-proof gate (#529, criterion 6).
A controller (or any session) may close a tracking issue with a closure
report that summarizes validation. When that report calls a non-zero
test-suite exit an "expected pre-existing failure" (or baseline / known
failure) it must carry pre-merge proof; otherwise the closure buries an
unproven regression as an accepted baseline and weakens controller
confidence in the durable state.
This module fails closed on exactly that case by reusing the pre-merge
baseline proof verifier (#533). A closure report is allowed when it is
empty (no validation claim), a clean pass, or a baseline failure proven on
the PR pre-merge base commit (or a documented known-failure record that
predates the PR).
"""
from __future__ import annotations
from typing import Any
from premerge_baseline_proof import CLEAN_PASS, assess_premerge_baseline_proof
def assess_controller_closure_baseline_proof(
closure_report: str | None,
) -> dict[str, Any]:
"""Assess whether an issue-closure report may proceed.
Returns a dict with ``block``, ``proven``, ``label``, ``reasons``,
``skipped`` and ``safe_next_action``. Only blocks when the closure
report claims a non-zero validation exit is an expected
pre-existing/baseline failure without valid pre-merge proof.
"""
text = (closure_report or "").strip()
if not text:
return {
"block": False,
"proven": True,
"label": CLEAN_PASS,
"reasons": [],
"skipped": True,
"safe_next_action": "",
}
result = assess_premerge_baseline_proof(text)
block = bool(result.get("block"))
return {
"block": block,
"proven": not block,
"label": result.get("label"),
"reasons": list(result.get("reasons") or []),
"skipped": bool(result.get("skipped", False)),
"safe_next_action": (
result.get("safe_next_action")
or (
"provide pre-merge baseline proof (base commit, tested commit, "
"command, exit status, failure signature) before closing on an "
"expected pre-existing failure"
)
)
if block
else "",
}
@@ -0,0 +1,63 @@
# Control-plane DB substrate (#613)
**Status:** Implemented (SQLite single-writer MVP)
**ADR:** [`mcp-allocator-control-plane-observability-adr.md`](mcp-allocator-control-plane-observability-adr.md)
**Module:** `control_plane_db.py`
## Architecture statement
> **DB coordinates, Gitea records, Sentry/GlitchTip observe; the bridge is the only path that turns observations into Gitea work.**
## What this ships
| Capability | Notes |
|------------|--------|
| Schema | `sessions`, `work_items`, `leases`, `assignments`, `terminal_locks`, `events`, `incident_links` |
| Atomic assign+lease | `ControlPlaneDB.assign_and_lease` — one `BEGIN IMMEDIATE` transaction |
| Mutation gate | `require_valid_assignment` — live lease + allowed action + non-terminal work + non-stale head |
| Heartbeat / release / expire | Lease lifecycle helpers |
| Terminal-lock index | Routing signal for #600 (terminal path first) |
| `incident_links` | Provider-neutral link model for #612**not** assignable work; scope keys NULL-safe |
## Hard rules (enforced in code)
1. Assignable `work_items.kind` ∈ {`issue`, `pr`} only — **never** raw Sentry/GlitchTip incidents.
2. Two concurrent sessions cannot both receive an active assignment on the same open work item (second gets `wait`).
3. Merged/closed work items return `no_safe_work` at assign time, and `require_valid_assignment` fails closed if the work item becomes terminal later.
4. Assignments pin `expected_head_sha`; mutations fail closed if the work item head drifts.
5. SQLite path is the **single-writer MVP** (`GITEA_CONTROL_PLANE_DB`, default under `~/.cache/gitea-tools/control-plane/`). Multi-session multi-host production requires **Postgres** or a **single allocator daemon** (ADR §6).
6. `incident_links` optional scope fields are stored as empty strings (never NULL) so UNIQUE is canonical across minimal upserts.
7. Legacy `incident_links` migration collapses NULL-scope duplicates **only** when Gitea targets **and** all meaningful observation metadata agree (fingerprint, status, event_count, permalink, timestamps, linked PRs, etc.). Conflicting metadata fails closed — no silent discard.
## Dependency chain
```text
#613 control-plane DB (this) → #600 allocator API → #612 incident bridge
```
- **#600** must call this substrate (not file locks / comment-only leases alone) for completion.
- **#612** must write `incident_links` here and create **Gitea issues**; the allocator assigns those issues, not raw incidents.
## Allocator API (#600)
Module: `allocator_service.py` · MCP tool: `gitea_allocate_next_work`
- Workers call `gitea_allocate_next_work(apply=false|true)` instead of self-selecting work.
- `apply=false` returns a dry-run selection (`outcome=preview`) with skip reasons.
- `apply=true` reserves via `ControlPlaneDB.assign_and_lease` (atomic assignment+lease).
- Coordination source is **always** the control-plane DB — not file locks or comment-only leases.
- Routing follows ADR §5.3 (REQUEST_CHANGES → author, terminal path first, foreign lease → wait).
- Raw monitoring incidents are never candidates (#612 remains downstream).
## Non-goals (intentionally deferred)
- Sentry/GlitchTip provider adapters and auto-watchdog — **#612**
- Gitea comment/label mirror writers (may be added by allocator tooling later)
## Tests
```bash
python3 -m pytest tests/test_control_plane_db.py -q
```
@@ -0,0 +1,301 @@
# ADR: MCP allocator, control-plane DB, and Sentry/GlitchTip incident bridge architecture
- **Status:** Accepted (implementation pending; blocks code for #600 / #612 / #613)
- **Date:** 2026-07-09
- **Tracking issues:**
- [#613](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/613) — control-plane DB (first)
- [#600](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/600) — allocator API (second)
- [#612](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/612) — Sentry/GlitchTip incident bridge (third)
- **Related:** existing GlitchTip contracts (`glitchtip-to-gitea-workflow-design.md`, `glitchtip-gitea-deduplication-linking-design.md`), trust boundaries (`tool-boundaries.md`, `safety-model.md`), current leases (`pr_work_lease.py`, `issue_lock_store.py`)
## 1. Context
Multiple LLM sessions can independently inspect the Gitea queue and start the same issue or PR. Existing coordination (Gitea comment leases, local issue-lock files, labels) often detects collisions **after** work has started. Parallel issues #600, #612, and #613 each describe part of a fix; without one ADR they risk overlapping stores, wrong dependency order, and trust-boundary violations.
This ADR is the canonical architecture decision **before** implementing those issues.
## 2. Decision summary (core)
| Layer | Owns | Must not |
|-------|------|----------|
| **Control-plane DB** | Sessions, atomic assignment, leases, heartbeats, terminal-lock index, events, incident links | Bypass Gitea workflow gates or replace issue/PR history |
| **Gitea** | Durable work record: issues, PRs, comments, labels, reviews, merges | Be the only concurrency lock under multi-session load |
| **Sentry / GlitchTip** | Incidents, events, provider UI | Assign work, approve/merge/close, or mutate Gitea outside the bridge |
| **Incident bridge** | Provider adapters, reconcile/create Gitea issues, link storage upsert, optional provider writeback | Hand raw provider incidents to the allocator as work items |
**One-liner:** **DB coordinates. Gitea records. Sentry/GlitchTip observe. The bridge is the only path that turns observations into Gitea work. The allocator assigns only Gitea issues/PRs, never raw monitoring incidents.**
## 3. Dependency order
Implementation **must** follow:
```text
#613 control-plane DB → #600 allocator API → #612 incident bridge
(atomic substrate) (routing policy) (feeds Gitea work)
```
| Issue | Role | Depends on |
|-------|------|------------|
| **#613** | Durable coordination DB + lease/assignment transactions | — |
| **#600** | `gitea_allocate_next_work` policy and tool surface | **#613** (hard) |
| **#612** | Multi-project Sentry/GlitchTip → Gitea bridge | **#613** for `incident_links` index; **#600** before treating bridge-created issues as allocator feed in multi-worker prod |
**Hard rules:**
1. Do **not** implement #600 on file locks / comment-only leases and call it done.
2. Do **not** ship #612 as a second assignment system for raw incidents.
3. Partial previews (read-only list tools, schema stubs) may land earlier if they do not claim “allocator complete” or “bridge complete.”
## 4. Authority boundaries
### 4.1 Gitea (durable source of truth for work)
Gitea remains authoritative for:
- Issue and PR identity, titles, bodies, state (open/closed/merged)
- Comments, reviews, approvals, REQUEST_CHANGES
- Labels and workflow status labels (`status:ready`, `status:in-progress`, …)
- Merges, closes, branch refs as recorded by Gitea/git hosting
Operators and auditors read **history** from Gitea.
### 4.2 Control-plane DB (coordination / index)
The control-plane DB is authoritative for **live multi-session coordination**:
- Which session holds which assignment/lease
- Heartbeat freshness and expiry
- Terminal-lock index for routing
- Event log of allocation/lease transitions
- `incident_links` index (see §8)
It is **not** a substitute for Gitea history. When DB and Gitea disagree on durable work state (e.g. PR already merged), **Gitea wins**; the DB is reconciled.
### 4.3 Sentry / GlitchTip (observe)
Providers own incident lifecycle and raw event data. They:
- Must **not** bypass Gitea gates
- Must **not** assign LLM work
- May receive **writeback** of resolution status only through the bridge, when configured and supported
### 4.4 Trust boundaries for credentials
Aligned with `docs/tool-boundaries.md` and `docs/safety-model.md`:
- **One MCP server process per trust boundary** remains the default.
- Monitor API tokens (Sentry/GlitchTip) live in the **bridge/orchestrator boundary** (or a dedicated observability write/read profile), **not** blindly inside every Gitea MCP author/reviewer/merger process.
- Gitea tokens stay on Gitea MCP profiles only.
- Orchestrators compose services; they must not become a single credential pool that silently mixes Gitea write + monitor tokens into every worker.
Tool names such as `gitea_observability_*` may exist as a **namespace façade** only if the runtime still enforces separate credential scopes (e.g. bridge process vs pure Gitea mutation process).
## 5. Allocator rules (#600 on top of #613)
### 5.1 Worker contract
- Workers **do not self-select** work under the standard multi-LLM workflow.
- Workers call **`gitea_allocate_next_work`** (with `apply=true` only when taking work).
- Mutations that claim exclusive work require a **valid assignment and lease** from the control-plane DB.
- Return values include at least: role, issue/PR number, expected head SHA (when applicable), lease ID, expiry, allowed actions, forbidden actions — or a non-assignment outcome (`WAIT`, terminal-path block, no safe work, needs controller, etc.).
### 5.2 Atomic reserve
- **Assignment creation and lease creation happen in one DB transaction.**
- Two concurrent sessions **must not** receive the same issue/PR as assigned work.
- On contention: second session gets **WAIT** or **owner-resume**, never a duplicate lease.
### 5.3 Routing policy
| Condition | Route |
|-----------|--------|
| Current-head **REQUEST_CHANGES** | **Author** (not reviewer/merger) |
| **Stale** approval (approval not on current head) | **Reviewer** |
| **Clean** approval on current head, mergeable | **Merger** |
| Contested / contaminated approval | **Diagnosis / reconciler** (controller path) |
| Active **foreign** lease | **WAIT** or **owner-resume** |
| **Terminal-review** lock present | **Terminal-path resolution first** before downstream review work |
| Merged / closed PR | **Never assign** |
| Issue locked by sanctioned lease | **Never assign** to another worker unless lease cleanup/adoption is sanctioned |
### 5.4 Relationship to Gitea mirrors
After a successful assignment, the allocator (or sanctioned tooling) may update Gitea comments/labels so humans and audits see state. Those mirrors **are not** the sole lock source.
## 6. Control-plane DB topology (#613)
### 6.1 Preferred architecture (multi-daemon / Proxmox)
For true multi-session concurrency (multiple MCP daemons, multiple hosts, or Proxmox VMs):
**Prefer either:**
1. **Shared Postgres** used by all Gitea MCP profiles / allocator callers, **or**
2. A **single allocator daemon** (sole writer to the coordination store) that all workers call over MCP/RPC.
Both satisfy: one transactional authority for “who has this work item.”
### 6.2 SQLite MVP
SQLite is acceptable **only** for a **single-writer MVP**:
- One process owns writes (allocator daemon or single co-located MCP server), **or**
- Documented single-host single-daemon experiments with file locking and no multi-host claims.
SQLite is **not** sufficient to declare multi-session production readiness when four independent LLM sessions talk to four MCP processes without a shared writer.
### 6.3 Suggested entities (normative shape)
Minimum logical entities (names may vary; semantics must not):
1. **sessions** — session_id, role/profile, namespace, pid, started_at, last_heartbeat_at, status
2. **work_items** — remote/org/repo, kind ∈ {`issue`, `pr`}, number, state, priority, current_head_sha, updated_at
3. **leases** — lease_id, work_item_id, session_id, role, phase, expires_at, heartbeat_at, status
4. **assignments** — assignment_id, work_item_id, session_id, allowed_action(s), expected_head_sha, status
5. **terminal_locks** — repo/org, terminal_pr, review_id, decision, status, cleanup_state
6. **events** — event_id, work_item_id, type, message, created_at
7. **incident_links** — provider-neutral link model (see §8)
**Explicit non-kind:** do **not** use `work_items.kind = sentry_incident` (or glitchtip_incident) as an assignable work unit. Incidents become work only after the bridge creates/links a **Gitea issue**.
## 7. Lease migration (avoid split-brain)
### 7.1 Target state
- **Primary** for live coordination: **control-plane DB** leases and assignments.
- **Gitea comment leases** (e.g. `<!-- mcp-review-lease:v1 -->`) and **local issue-lock files** become:
- **mirrors** of DB state for human visibility / recovery, and/or
- written **only** through the allocator (or sanctioned lease tools that update DB + mirror in one workflow).
### 7.2 Migration rules
1. New exclusive claims go through DB assignment+lease first.
2. Comment lease / file lock writers that skip the DB are **deprecated** once #613+#600 land.
3. Reconciler tooling heals: expired DB lease, orphan Gitea comment, orphan local file — fail closed when ambiguous.
4. **Split-brain is a defect:** “DB free + Gitea comment leased” or “DB leased + Gitea free” must be detectable and reconcilable; production paths must not rely on two independent writers.
### 7.3 Heartbeats
- Heartbeats update the **DB**.
- Optional Gitea summaries must avoid comment spam (periodic summary or edit-in-place policy).
## 8. Observability bridge (#612)
### 8.1 Role
The bridge:
1. Scans configured Sentry and/or GlitchTip projects (multi-project mappings).
2. Deduplicates against existing links / Gitea issues.
3. Creates or updates **normal Gitea issues** (labels, sanitized body, provider URL).
4. Upserts **one** canonical `incident_links` row.
5. Optionally writes resolution status back to the provider when supported.
6. Never approves, merges, closes, or otherwise bypasses Gitea workflow gates.
### 8.2 Allocator interaction
- The allocator sees observability work **only after** a Gitea issue exists (typically `status:ready` + observability labels).
- **Do not assign raw Sentry/GlitchTip incidents** as work items.
- Bridge AC “allocator can see linked issues” means: **as ordinary Gitea issues**, not a special incident queue.
### 8.3 Provider adapters and trust
- Separate **Sentry** and **GlitchTip** adapters; document API differences; do not assume writeback parity.
- Self-hosted base URLs supported (e.g. `https://sentry.prgs.cc`).
- Tokens from environment / secret store only.
- Prefer phase-1 **explicit reconcile / dry-run** before unsupervised watchdog auto-filing, consistent with existing GlitchTip filing safety contracts.
### 8.4 Home of implementation
Filing/orchestration may live in:
- a control-plane / bridge package or profile, and/or
- Gitea-Tools as operator-facing tools that **delegate** to that boundary,
but must not violate §4.4 credential isolation.
## 9. Incident link storage (single source of truth)
### 9.1 Canonical model: `incident_links` (provider-neutral)
Prefer a **provider-neutral** model over Sentry-only `sentry_links`:
| Field (logical) | Purpose |
|-----------------|---------|
| provider | `sentry` \| `glitchtip` \| … |
| provider_base_url | Self-hosted base |
| provider_org / provider_project | Mapping key |
| provider_issue_id / provider_short_id | Provider identity |
| provider_permalink | Human URL |
| fingerprint | Stable dedupe key when available |
| gitea_org / gitea_repo / gitea_issue_number | Linked work |
| linked_pr_numbers | Optional PR association |
| first_seen / last_seen / event_count | Summary metrics (safe) |
| status | link lifecycle |
| release_resolved_at / last_sync_at | Sync bookkeeping |
### 9.2 Gitea as human mirror
- Issue body markers / structured comments (e.g. HTML comment metadata) remain the **human- and audit-visible mirror**.
- They must stay consistent with `incident_links` but are **not** a second independent write path for inventing links without the bridge.
### 9.3 One truth
- **Do not** maintain two independent systems of record (e.g. full mapping only in #612 storage **and** a separate #613 `sentry_links` with different semantics).
- #612 implementation **must** use the same `incident_links` model introduced under #613 (or a single agreed store both reference).
## 10. Security and redaction
Mandatory for bridge payloads, Gitea issue bodies/comments, DB-stored event summaries, and logs:
- No API tokens, DSNs, passwords, cookies, auth headers
- No keychain IDs, private config contents, raw session-state files, full prompt bodies
- Sanitize stack locals and request data; store only safe tags/context
- Logs must never print provider tokens or DSNs
- Redaction tests are required for #612
## 11. Non-goals
- Replace Gitea as the durable workflow record
- Let Sentry/GlitchTip assign work or mutate Gitea workflow state outside sanctioned tools
- Let the bridge approve, merge, close, release, or bypass Gitea gates
- Implement #600 on file locks / comments alone and call allocator complete
- Treat raw monitoring incidents as `work_items` for the allocator
- Put monitor tokens into every Gitea MCP worker process by default
## 12. Consequences
### Positive
- Clear implementation order and ownership
- Multi-session safety becomes testable at the DB layer
- Observability becomes assignable work without special-casing the allocator
- Trust boundaries stay compatible with existing control-plane docs
### Costs / risks
- Migration from comment/file leases requires reconciler work
- Shared Postgres or single allocator daemon is operational cost
- Bridge must be careful not to spam Gitea issues (dedupe, caps, policy modes)
### Follow-ups (documentation only until implemented)
1. Update #600, #612, and #613 bodies to **link this ADR** and restate hard dependencies.
2. Implement #613 schema + atomic assign/lease.
3. Implement #600 against the DB.
4. Implement #612 against `incident_links` + Gitea create/update.
5. Deprecate dual writers for leases.
## 13. Acceptance criteria for *this* ADR
This document is accepted when:
1. It is merged into `docs/architecture/` on the default branch.
2. #600 / #612 / #613 (or their PRs) reference this path as the architecture source of truth.
3. No implementation PR for those issues claims completion without conforming to §§211.
## 14. Document history
| Date | Change |
|------|--------|
| 2026-07-09 | Initial ADR: dependency order, authority model, topology, lease migration, bridge, `incident_links`, security, non-goals |
+92
View File
@@ -811,6 +811,32 @@ author work. Do not switch from `prgs-reconciler` to `prgs-author` only to run
- **Reports:** label cleanup actions as reconciler cleanup (not author mutation). - **Reports:** label cleanup actions as reconciler cleanup (not author mutation).
- **Prompt:** `As prgs-reconciler, dry-run then execute gitea_reconcile_merged_cleanups for recently merged PRs without switching to prgs-author.` - **Prompt:** `As prgs-reconciler, dry-run then execute gitea_reconcile_merged_cleanups for recently merged PRs without switching to prgs-author.`
### Superseded PR / satisfied issue reconciliation (#525)
Closing duplicate or superseded PRs after a canonical PR has merged is
**reconciler** work. Do not switch to `prgs-author` only to close the duplicate
PR, close the satisfied issue, or file a narrow follow-up discovered during
reconciliation.
- **Profile:** `prgs-reconciler`.
- **Tasks:** `reconcile_close_superseded_pr`,
`reconcile_close_satisfied_issue`, and
`reconcile_create_followup_issue`.
- **Tool:** `gitea_reconcile_superseded_by_merged_pr`.
- **Required proof:**
1. Live target PR state is open, but it is not independently required and no
mergeable work remains.
2. Live superseding PR state is closed and merged.
3. Superseding PR head is an ancestor of freshly fetched `master`.
4. Merge commit SHA is recorded.
5. Canonical close comment cites the superseding PR and merge commit.
6. Linked issue closure is attempted only when the issue is open and
explicitly satisfied by the superseding PR.
- **Fail closed:** missing ancestry proof, missing canonical comment,
mergeable target PR, same target/superseding PR, or missing
`gitea.pr.close` / `gitea.issue.close` capability.
- **Prompt:** `As prgs-reconciler, reconcile PR #N as superseded by merged PR #M; post the canonical close comment, close the superseded PR, and close issue #K only if the merged PR fully satisfies it.`
### Stop on blocker ### Stop on blocker
- **Any profile.** If a required gate cannot be satisfied — identity - **Any profile.** If a required gate cannot be satisfied — identity
@@ -1023,6 +1049,72 @@ Special states:
Final reports must not claim a comment was posted when Final reports must not claim a comment was posted when
`canonical_comment_validation.allowed` is false (#496 AC14). `canonical_comment_validation.allowed` is false (#496 AC14).
## Stale #332 review-decision lock cleanup (#594)
#332 hard-stops a reviewer session after a terminal live review mutation
(`approve` / `request_changes`). After #559 those locks are **durable** under
`~/.cache/gitea-tools/session-state/review_decision_lock-<profile>.json`, so they
can outlive the PR they protected.
### When cleanup is **allowed**
Use `gitea_cleanup_stale_review_decision_lock` from a **reviewer** profile when:
1. A durable review-decision lock has a terminal live mutation, **and**
2. Live Gitea state shows that terminal PR is already **merged** or **closed**
(so no same-PR merge sequence remains), **and**
3. The active profile identity matches the lock, **and**
4. Authenticated identity can be verified.
Workflow:
```text
# 1) Assess only (default)
gitea_cleanup_stale_review_decision_lock(apply=false, remote=prgs, ...)
# 2) Apply after is_moot=true and cleanup_allowed=true
gitea_cleanup_stale_review_decision_lock(
apply=true,
expected_terminal_pr=<merged-or-closed-pr>,
remote=prgs,
...
)
```
Successful apply:
* clears in-memory + durable decision lock for that profile
* returns a structured `audit` payload
* posts an audit comment on the mooted PR when `gitea.pr.comment` is allowed
(`post_audit_comment=true` default)
Same-profile auto-expire: if `gitea_merge_pr` succeeds on a PR that this same
profile just approved, the decision lock is cleared after merge. Cross-profile
locks (reviewer vs merger) still require the cleanup tool.
### When cleanup is **forbidden**
Do **not** clear when:
* the last terminal PR is still **open** (active #332 hard-stop — continue merge
for that approve, or stop after request_changes)
* live PR state cannot be fetched (fail closed)
* profile identity does not match the lock
* identity cannot be verified
* `expected_terminal_pr` does not match the last terminal PR
**Never** use manual deletion of session-state files as the normal workflow.
**Never** use cleanup to skip review of an open PR or to bypass eligibility /
mark-ready / submit gates on active work.
### Related tools
| Tool | Purpose |
|------|---------|
| `gitea_cleanup_stale_review_decision_lock` | Moot decision-lock cleanup (#594) |
| `gitea_authorize_review_correction` | Operator-approved correction after a **mistaken** live review on still-active work (#211) |
| `gitea_cleanup_post_merge_moot_lease` | Moot **lease** cleanup after merge (#515) — different object |
## Fail-closed behavior ## Fail-closed behavior
Before any mutating action the workflow verifies identity, active profile, Before any mutating action the workflow verifies identity, active profile,
+118
View File
@@ -0,0 +1,118 @@
# Recovering from `client is closing: EOF` on a Gitea MCP namespace (#543)
## Symptom
A tool call through a Gitea MCP namespace — `gitea-author`, `gitea-reviewer`,
`gitea-merger`, or the shared `gitea-tools` namespace — fails immediately with:
```
client is closing: EOF
```
Every subsequent call to that same namespace returns the same error, including
cheap read tools such as `gitea_whoami` and `gitea_list_profiles`. Other MCP
servers registered with the same client (for example `context7`) keep working,
so this is **not** a global MCP-client outage.
## Why this is not a code defect
This failure is a **transport-level** condition in the IDE / MCP client manager,
not a missing or broken tool:
- The tool can be present and registered in the Python `FastMCP` tool manager.
- Direct Python inspection of the server confirms the tool exists.
- Running the server manually and sending JSON-RPC over stdio works fine
(offline spawn) — that path does **not** prove the IDE namespace is healthy.
The client manager entered a closed state after the backing subprocess for that
namespace terminated (or was killed) behind its back. Once closed, the client
does **not** re-spawn the child on the next tool call — it just replays
`client is closing: EOF`. The OS process may even still be alive if a parent
language-server process is holding the stdio pipes open.
This is the canonical "registered in FastMCP ≠ callable through the namespace"
false-ready state. It is distinct from the **stale-runtime** family in #531 /
#544, where the process is reachable but running behind `master`; that case is
detected by the `ps`-based `_check_mcp_runtimes_diagnostics` in
`gitea_mcp_server.py`. The EOF case is a dead/closed transport, not a stale one,
so the `ps` check alone will not surface it.
## Recovery path (canonical — client reconnect only)
Do the steps in order. Stop as soon as a live **client-namespace** call succeeds.
1. **Confirm the blast radius.** Call a cheap read tool on the failing namespace
(`gitea_whoami` or `gitea_list_profiles`). Then call the same tool on a
different MCP server (e.g. `context7`).
- Only the Gitea namespace fails → single-namespace transport close. Continue.
- Every server fails → restart the whole MCP client, not just one namespace.
2. **Reconnect the namespace through the client, not the shell.** Use the IDE /
client MCP-reconnect action for that server entry (in Claude Code:
`/mcp` → reconnect the affected `gitea-*` server). Reconnecting forces the
client to spawn a fresh subprocess and re-open the pipe. This clears the
closed-client state that a bare `kill`/respawn from a terminal does **not**.
3. **Do not "fix" it by importing the server or poking the process.** Reaching
for `python -c 'import gitea_mcp_server ...'`, raw JSON-RPC from a shell,
killing PIDs to force a respawn, or touching MCP config mtimes does **not**
restore the *client's* view of the namespace and violates the daemon-import
guard (#558, `docs/mcp-daemon-import-guard.md`). The only sanctioned repair
is a **client reconnect / relaunch**.
4. **Verify through the same path the workflow will use.** After reconnect, call
the specific tool the blocked workflow needs — not just any tool — through
the target namespace. For a merge that means calling the merger-authorized
adoption/merge tool through `gitea-merger`. A green `gitea_whoami` on one
namespace does **not** prove another namespace or another tool is callable.
Record success with:
```text
gitea_assess_mcp_namespace_health(..., probe_source="client_namespace")
```
5. **If reconnect does not clear it,** relaunch the client entirely, then repeat
step 4. If EOF persists after a full relaunch, the backing subprocess is
failing to start — inspect its stderr / launch config (command path, venv,
`*_MCP_CONFIG`, `*_MCP_PROFILE` env) rather than retrying the call. Still
do not use PID kill or config-touch as the primary recovery.
## Diagnostics to capture when reporting EOF
Include all of these so the failure is actionable and reproducible:
- **Namespace name** that returned EOF (`gitea-author` / `gitea-reviewer` /
`gitea-merger` / `gitea-tools`).
- **Tool** that was called and the **exact** error string.
- **PID** of the backing process (if any) and whether it was still alive
(informational only — not a recovery action).
- **Profile / env** for that namespace (execution profile, `*_MCP_PROFILE`,
worktree binding such as `GITEA_AUTHOR_WORKTREE`).
- **Config path** the client launched the server from.
- Result of the **cross-server control** call (did `context7` succeed?).
## Offline spawn probe (non-authoritative)
`test_mcp_conn.py` performs a full JSON-RPC handshake against a **fresh
subprocess** (`initialize` → `initialized` → `tools/list` → `tools/call`) and
classifies with `probe_source=offline_spawn`. That is useful for offline
launch/registration debugging. It is **not** proof the IDE-managed namespace is
healthy. See `docs/mcp-namespace-health.md`.
## Do-not list during EOF recovery
- Do **not** retry a blocked merge/adoption until the required tool is confirmed
callable through the merger-authorized **client** namespace (see #543).
- Do **not** clean, reset, or rebind a **foreign** worktree to work around the
error.
- Do **not** bypass the namespace with direct imports, raw API/curl, or
in-memory state restoration.
- Do **not** kill MCP PIDs or touch config mtimes as a substitute for client
reconnect.
## Related
- #531 / #544 — stale-runtime detection (`ps`-based); sibling failure mode.
- #558 / `docs/mcp-daemon-import-guard.md` — why shell imports are not a repair.
- `docs/mcp-client-registration.md` — per-server registration contract.
- `docs/mcp-namespace-health.md` — probe sources and mutation enforcement.
+88
View File
@@ -0,0 +1,88 @@
# MCP namespace health diagnostics (#543)
Gitea MCP tools can be registered in the Python FastMCP server while the IDE's
live MCP namespace is still unusable. The failure usually appears as
`client is closing: EOF`, `transport closed`, or an empty response when calling
a tool such as `gitea_whoami`.
Do not treat static tool registration as proof that review or merge workflows
can proceed. A reviewer or merger flow must have **client-namespace** evidence
that the required tool is callable through the configured IDE MCP namespace.
## Probe sources (do not confuse them)
| Source | How obtained | Proves IDE namespace? |
| --- | --- | --- |
| `client_namespace` | Tool call through the IDE-managed MCP client | **Yes** |
| `offline_spawn` | `test_mcp_conn.py` subprocess JSON-RPC handshake | **No** (offline only) |
`gitea_assess_mcp_namespace_health` accepts `probe_source` and only sets
`ide_namespace_proven=true` for `client_namespace` success. Offline spawn
success never clears review/merge mutation gates.
## Client-namespace health check (canonical)
1. Through the IDE client, call a cheap tool on the target namespace
(`gitea_whoami` or `gitea_list_profiles`).
2. Feed the live result into:
```text
gitea_assess_mcp_namespace_health(
namespace="gitea-merger", # or reviewer / author / tools
registered_tools=[...], # optional static list
probe_result={"success": true, "result": {...}},
probe_source="client_namespace",
)
```
3. A healthy client-namespace assessment is recorded in the MCP session and
consulted by **live** `gitea_submit_pr_review` / `gitea_merge_pr` gates.
4. If the probe fails with EOF, recover via **client reconnect only** — see
`docs/mcp-namespace-eof-recovery.md`. Do **not** kill PIDs or touch MCP
config mtimes as a recovery procedure.
## Offline spawn probe (non-authoritative)
```bash
python3 test_mcp_conn.py --config ~/.gemini/config/mcp_config.json
```
This script spawns a **separate** server process from config, performs
JSON-RPC `initialize``tools/list``tools/call`, and classifies the
result with `probe_source=offline_spawn`. Use it for offline debugging of
launch command / registration. It does **not** prove the IDE-managed
namespace is healthy.
By default the script checks:
| Namespace | Required tool |
| --- | --- |
| `gitea-author` | `gitea_whoami` |
| `gitea-reviewer` | `gitea_whoami` |
| `gitea-merger` | `gitea_whoami` |
| `gitea-tools` | `gitea_list_profiles` |
## Recovery (canonical)
When a namespace returns EOF, follow
`docs/mcp-namespace-eof-recovery.md` in order:
1. Confirm blast radius (Gitea namespace vs all MCP servers).
2. **Reconnect the namespace through the client** (IDE reconnect / relaunch).
3. Do not repair via shell imports, raw JSON-RPC, PID kills, or config mtime
touches — those do not restore the client's closed transport.
4. Re-verify the **specific** required tool through the target namespace.
5. Resume review/merge only after a successful `client_namespace` assessment.
## Enforcement
1. **State machine (read-only):** feed `blocks_merge_workflow` from a
`client_namespace` assessment into
`gitea_assess_review_merge_state_machine(live_namespace_broken=...)`.
2. **Live mutations:** `gitea_submit_pr_review` and `gitea_merge_pr` call
`_live_namespace_health_gate` and fail closed when the session has a
recorded unhealthy or non-client probe for the required namespace
(`gitea-reviewer` for review, `gitea-merger` for merge).
When blocked, repair the IDE namespace and re-record a healthy
`client_namespace` assessment before retrying the mutation.
+4 -2
View File
@@ -15,5 +15,7 @@
5. **Explicit merge confirmation**`gitea_merge_pr` requires `confirmation="MERGE PR <n>"`. 5. **Explicit merge confirmation**`gitea_merge_pr` requires `confirmation="MERGE PR <n>"`.
6. **No self-review / self-merge** — Authenticated user must differ from PR author for approve/merge. 6. **No self-review / self-merge** — Authenticated user must differ from PR author for approve/merge.
7. **Review decision lock** — Live review mutations require validation-phase dry-run and `gitea_mark_final_review_decision`. 7. **Review decision lock** — Live review mutations require validation-phase dry-run and `gitea_mark_final_review_decision`.
8. **Redaction** — Tokens, passwords, and keychain material never appear in tool output. 8. **Terminal review hard-stop (#332)** — After a terminal live review mutation, only same-PR merge after `approve` may continue. Durable locks (#559) must not be deleted by hand.
9. **Wiki publication (#224)**`docs/wiki/` and the sync helper are prerequisites only. Closing a wiki issue requires live Gitea Wiki proof on the repo Wiki tab. See [Runbooks](Runbooks.md#wiki-publication-readiness-gate-224). 9. **Stale decision-lock cleanup (#594)**`gitea_cleanup_stale_review_decision_lock` may clear a durable #332 lock **only** when the last terminal mutation's PR is live-state **merged or closed**, identity/profile gates pass, and apply uses a reviewer-capable profile. Open/ambiguous locks stay fail-closed. Successful cleanup records a durable audit trail.
10. **Redaction** — Tokens, passwords, and keychain material never appear in tool output.
11. **Wiki publication (#224)**`docs/wiki/` and the sync helper are prerequisites only. Closing a wiki issue requires live Gitea Wiki proof on the repo Wiki tab. See [Runbooks](Runbooks.md#wiki-publication-readiness-gate-224).
+54
View File
@@ -14,6 +14,7 @@ from typing import Any, Callable
import branch_cleanup_guard import branch_cleanup_guard
import issue_acceptance_gate import issue_acceptance_gate
import issue_lock_provenance import issue_lock_provenance
import merger_lease_adoption
import reviewer_handoff_consistency import reviewer_handoff_consistency
import thread_state_ledger_validator import thread_state_ledger_validator
from mcp_native_cleanup_proof import assess_mcp_native_cleanup_proof from mcp_native_cleanup_proof import assess_mcp_native_cleanup_proof
@@ -39,6 +40,7 @@ FINAL_REPORT_TASK_KINDS = frozenset({
"issue_filing", "issue_filing",
"issue_selection", "issue_selection",
"inventory", "inventory",
"controller_close",
}) })
_TASK_KIND_ALIASES = { _TASK_KIND_ALIASES = {
@@ -50,6 +52,9 @@ _TASK_KIND_ALIASES = {
"reconcile_already_landed": "reconcile_already_landed", "reconcile_already_landed": "reconcile_already_landed",
"work-issue": "work_issue", "work-issue": "work_issue",
"create_issue": "issue_filing", "create_issue": "issue_filing",
"close_issue": "controller_close",
"close-issue": "controller_close",
"controller-close": "controller_close",
} }
_HANDOFF_ROLE_BY_TASK = { _HANDOFF_ROLE_BY_TASK = {
@@ -61,6 +66,7 @@ _HANDOFF_ROLE_BY_TASK = {
"issue_filing": "issue_filing", "issue_filing": "issue_filing",
"issue_selection": None, "issue_selection": None,
"inventory": "inventory", "inventory": "inventory",
"controller_close": None,
} }
_LEGACY_WORKSPACE_MUTATIONS_RE = re.compile( _LEGACY_WORKSPACE_MUTATIONS_RE = re.compile(
@@ -849,6 +855,27 @@ def _rule_reviewer_premerge_baseline_proof(report_text: str) -> list[dict[str, s
) )
def _rule_reviewer_post_merge_validation(report_text: str) -> list[dict[str, str]]:
"""Block active approval on an already-merged/closed PR, and post-merge moot
validation claimed without merged-state + merge-commit proof (#529)."""
from post_merge_validation import assess_post_merge_validation
result = assess_post_merge_validation(report_text)
if not result.get("block"):
return []
return _findings_from_reasons(
"reviewer.post_merge_validation",
result.get("reasons") or [],
field="Validation status",
severity="block",
safe_next_action=result.get("safe_next_action")
or (
"record post-merge moot validation instead of an active approval; "
"cite PR state merged/closed and the merge commit SHA"
),
)
def _rule_reviewer_validation_status_vocabulary( def _rule_reviewer_validation_status_vocabulary(
report_text: str, report_text: str,
*, *,
@@ -1263,6 +1290,24 @@ def _rule_shared_manual_lock_pr_override(report_text: str) -> list[dict[str, str
) )
def _rule_shared_manual_lease_proof_handoff(report_text: str) -> list[dict[str, str]]:
"""Block merge/review handoffs that claim unsafe lease seeding (#535)."""
result = merger_lease_adoption.assess_manual_lease_proof_handoff(report_text)
if result.get("proven"):
return []
return _findings_from_reasons(
"shared.manual_lease_proof_handoff",
result.get("reasons") or [],
field="Merge mutations",
severity="block",
safe_next_action=(
"use gitea_adopt_merger_pr_lease or gitea_acquire_reviewer_pr_lease; "
"cite lease_proof_source / adoption_comment_id; never claim manual "
"seeded/injected lease proof"
),
)
def _rule_shared_issue_acceptance_gate(report_text: str) -> list[dict[str, str]]: def _rule_shared_issue_acceptance_gate(report_text: str) -> list[dict[str, str]]:
result = issue_acceptance_gate.validate_final_report_issue_acceptance(report_text) result = issue_acceptance_gate.validate_final_report_issue_acceptance(report_text)
if not result.get("applicable") or result.get("valid"): if not result.get("applicable") or result.get("valid"):
@@ -1459,6 +1504,7 @@ def _rule_shared_mcp_native_cleanup_proof(report_text: str) -> list[dict[str, st
_SHARED_ISSUE_LOCK_RULES = ( _SHARED_ISSUE_LOCK_RULES = (
_rule_shared_issue_lock_external_state, _rule_shared_issue_lock_external_state,
_rule_shared_manual_lock_pr_override, _rule_shared_manual_lock_pr_override,
_rule_shared_manual_lease_proof_handoff,
_rule_shared_author_reviewer_same_run, _rule_shared_author_reviewer_same_run,
_rule_shared_raw_branch_delete_bypass, _rule_shared_raw_branch_delete_bypass,
_rule_shared_canonical_state_update, _rule_shared_canonical_state_update,
@@ -1494,6 +1540,7 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
_rule_reviewer_linked_issue, _rule_reviewer_linked_issue,
_rule_reviewer_baseline_on_failure, _rule_reviewer_baseline_on_failure,
_rule_reviewer_premerge_baseline_proof, _rule_reviewer_premerge_baseline_proof,
_rule_reviewer_post_merge_validation,
_rule_reviewer_validation_status_vocabulary, _rule_reviewer_validation_status_vocabulary,
_rule_reviewer_main_checkout_baseline, _rule_reviewer_main_checkout_baseline,
_rule_reviewer_main_checkout_path, _rule_reviewer_main_checkout_path,
@@ -1586,6 +1633,13 @@ _RULES_BY_TASK: dict[str, list[Callable[..., list[dict[str, str]]]]] = {
*_SHARED_CANONICAL_COMMENT_RULES, *_SHARED_CANONICAL_COMMENT_RULES,
*_SHARED_ISSUE_LOCK_RULES, *_SHARED_ISSUE_LOCK_RULES,
], ],
# Controller issue closure (#529): a closure report must not bury an
# unproven non-zero suite exit as an "expected pre-existing failure".
# Kept intentionally narrow so a closure pre-check does not demand the
# full reviewer/author handoff schema.
"controller_close": [
_rule_reviewer_premerge_baseline_proof,
],
} }
+1630 -66
View File
File diff suppressed because it is too large Load Diff
+180
View File
@@ -0,0 +1,180 @@
"""Pre-create issue content gate (#582).
Blocks issue creation when the title/body is a *vague reference* to
out-of-band content the LLM cannot prove it has ("the drafted issue",
"the prepared issue", "the issue we discussed", "the previous draft")
with no durable source pointer.
The rule from #582: an LLM must not fabricate an issue from stale chat
memory. When the requested issue content is a vague reference and no
durable source pointer (existing issue/PR/comment, checked-in file, URL,
or scratchpad path) is present, the gate fails closed and the caller must
return BLOCKED + DIAGNOSE naming the exact vague/missing fields.
This gate intentionally does NOT require a non-empty body: title-only
issue creation remains allowed for callers that explicitly want it. It
only blocks content that is a placeholder reference to something the
current context does not contain.
"""
from __future__ import annotations
import re
import unicodedata
# Vague reference phrases that point at out-of-band draft content. Stored
# normalized (lowercase, punctuation-stripped, whitespace-collapsed) so they
# can be matched against normalized field text.
VAGUE_REFERENCE_PHRASES: tuple[str, ...] = (
"the drafted issue",
"drafted issue",
"the prepared issue",
"prepared issue",
"the issue we discussed",
"the issue we talked about",
"the one we discussed",
"the previous draft",
"previous draft",
"the draft above",
"the issue above",
"as discussed",
"as we discussed",
"as previously discussed",
"per our discussion",
"per our conversation",
"per our chat",
"see above",
"same as before",
"the issue from earlier",
"the issue i drafted",
"the issue you drafted",
)
# A field that only restates a vague phrase (optionally with a leading verb
# such as "create"/"add"/"open"/"file") is still a vague reference.
_LEADING_VERBS = ("create", "add", "open", "file", "make", "please", "the")
# Durable source pointers: if any of these appears, the content points at a
# retrievable source of truth and is NOT treated as a fabricated reference.
_DURABLE_SOURCE_REGEXES: tuple[re.Pattern[str], ...] = (
re.compile(r"#\d+"), # #402
re.compile(r"\b(?:issue|pull|pr)\s+#?\d+", re.I), # issue 402 / PR #17
re.compile(r"\bcomment\s+#?\d+", re.I), # comment 8155
re.compile(r"https?://\S+", re.I), # URL
re.compile( # checked-in file
r"[\w./-]+\.(?:py|md|json|txt|sh|ya?ml|toml|cfg|ini|rst)\b", re.I
),
re.compile(r"\bscratchpad\b", re.I), # scratchpad path
re.compile(r"(?:^|\s)/[\w./-]+"), # absolute path
)
def normalize_text(text: str) -> str:
"""Lowercase, punctuation-stripped, whitespace-collapsed text."""
norm = unicodedata.normalize("NFKC", (text or "").strip().lower())
norm = re.sub(r"[^\w\s]", " ", norm)
norm = re.sub(r"\s+", " ", norm).strip()
return norm
def has_durable_source_pointer(text: str) -> bool:
"""True when the raw text references a durable, retrievable source."""
raw = text or ""
return any(rx.search(raw) for rx in _DURABLE_SOURCE_REGEXES)
def find_vague_reference(text: str) -> str | None:
"""Return the vague phrase a field is dominated by, else ``None``.
A field is a vague reference only when it contains a known vague phrase,
carries no durable source pointer, and the phrase dominates the field
(the field is essentially just the phrase). Long, specific bodies that
merely contain "as discussed" in passing are not blocked.
"""
if has_durable_source_pointer(text):
return None
norm = normalize_text(text)
if not norm:
return None
stripped = norm
for verb in _LEADING_VERBS:
if stripped.startswith(verb + " "):
stripped = stripped[len(verb) + 1:]
for phrase in VAGUE_REFERENCE_PHRASES:
if phrase in norm and len(stripped) <= len(phrase) + 12:
return phrase
return None
def assess_issue_content(
title: str,
body: str = "",
*,
allow_incomplete: bool = False,
) -> dict:
"""Evaluate whether proposed issue content is durable enough to create.
Returns a dict with ``complete`` (bool), ``missing_fields``,
``vague_fields``, ``reasons``, ``diagnose`` (joined reasons), and
``has_durable_source_pointer``. ``allow_incomplete`` is an explicit
operator override that forces ``complete`` True.
"""
t = (title or "").strip()
b = (body or "").strip()
missing_fields: list[str] = []
vague_fields: list[str] = []
reasons: list[str] = []
if not t:
missing_fields.append("title")
reasons.append("issue title is required")
else:
phrase = find_vague_reference(t)
if phrase:
vague_fields.append("title")
reasons.append(
f"title is a vague reference ('{phrase}') with no durable "
"content or source pointer"
)
if b:
phrase = find_vague_reference(b)
if phrase:
vague_fields.append("body")
reasons.append(
f"body is a vague reference ('{phrase}') with no durable "
"content or source pointer"
)
complete = allow_incomplete or (not missing_fields and not vague_fields)
return {
"complete": complete,
"missing_fields": missing_fields,
"vague_fields": vague_fields,
"reasons": reasons,
"diagnose": "; ".join(reasons),
"has_durable_source_pointer": has_durable_source_pointer(b)
or has_durable_source_pointer(t),
"override_applied": bool(
allow_incomplete and (missing_fields or vague_fields)
),
}
def pre_create_issue_content_gate(
title: str,
body: str = "",
*,
allow_incomplete: bool = False,
) -> dict:
"""Content gate wrapper mirroring the duplicate gate shape.
``performed`` is True when the content is durable enough to create (or an
explicit override was supplied). When False, the caller must return
BLOCKED + DIAGNOSE and must not create the issue.
"""
assessment = assess_issue_content(
title, body, allow_incomplete=allow_incomplete
)
assessment["performed"] = assessment["complete"]
return assessment
+27 -1
View File
@@ -42,12 +42,38 @@ STATUS_LABEL_SPECS: tuple[LabelSpec, ...] = (
LabelSpec("status:wontfix", "000000", "Issue will not be fixed"), LabelSpec("status:wontfix", "000000", "Issue will not be fixed"),
) )
# Durable validation-outcome labels (#529): the four canonical distinctions a
# reviewer report can carry. These are orthogonal to the single active status:*
# label, so they use their own prefix and are not subject to the one-status
# invariant.
VALIDATION_LABEL_SPECS: tuple[LabelSpec, ...] = (
LabelSpec("validation:clean-pass", "0e8a16", "Validation was a clean pass"),
LabelSpec(
"validation:baseline-accepted",
"fbca04",
"Validation passed with a baseline-proven unrelated failure",
),
LabelSpec(
"validation:blocked",
"b60205",
"Validation blocked by an unresolved failure",
),
LabelSpec(
"validation:post-merge-moot",
"5319e7",
"Validation is post-merge moot (PR already merged/closed before review)",
),
)
CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = ( CANONICAL_LABEL_SPECS: tuple[LabelSpec, ...] = (
TYPE_LABEL_SPECS + STATUS_LABEL_SPECS TYPE_LABEL_SPECS + STATUS_LABEL_SPECS + VALIDATION_LABEL_SPECS
) )
TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS) TYPE_LABELS: frozenset[str] = frozenset(spec.name for spec in TYPE_LABEL_SPECS)
STATUS_LABELS: frozenset[str] = frozenset(spec.name for spec in STATUS_LABEL_SPECS) STATUS_LABELS: frozenset[str] = frozenset(spec.name for spec in STATUS_LABEL_SPECS)
VALIDATION_LABELS: frozenset[str] = frozenset(
spec.name for spec in VALIDATION_LABEL_SPECS
)
CANONICAL_LABELS: frozenset[str] = frozenset( CANONICAL_LABELS: frozenset[str] = frozenset(
spec.name for spec in CANONICAL_LABEL_SPECS spec.name for spec in CANONICAL_LABEL_SPECS
) )
+306
View File
@@ -0,0 +1,306 @@
"""Assess live MCP namespace health without trusting static registration.
The IDE/client namespace can fail with EOF even when this Python process still
registers the Gitea tools with FastMCP. These helpers keep that distinction
explicit so reviewer/merger flows can fail closed on the live path.
Probe sources
-------------
* ``client_namespace`` — evidence from the IDE-managed MCP client path (the
only source that can prove the workflow namespace is healthy).
* ``offline_spawn`` — a separate ``subprocess.Popen`` JSON-RPC handshake
(e.g. ``test_mcp_conn.py``). Useful offline, but **never** proves the
IDE-managed namespace is callable.
* ``unknown`` — legacy/unspecified; treated as not IDE-proven.
"""
from __future__ import annotations
from typing import Any
REQUIRED_NAMESPACE_TOOLS = {
"gitea-author": "gitea_whoami",
"gitea-reviewer": "gitea_whoami",
"gitea-merger": "gitea_whoami",
"gitea-tools": "gitea_list_profiles",
}
DEFAULT_NAMESPACES = tuple(REQUIRED_NAMESPACE_TOOLS)
# Namespaces that must be healthy for a given mutation task.
TASK_REQUIRED_NAMESPACES = {
"review_pr": "gitea-reviewer",
"submit_review": "gitea-reviewer",
"merge_pr": "gitea-merger",
}
PROBE_SOURCE_CLIENT = "client_namespace"
PROBE_SOURCE_OFFLINE = "offline_spawn"
PROBE_SOURCE_UNKNOWN = "unknown"
VALID_PROBE_SOURCES = frozenset(
{PROBE_SOURCE_CLIENT, PROBE_SOURCE_OFFLINE, PROBE_SOURCE_UNKNOWN}
)
EOF_PATTERNS = (
"client is closing: eof",
"transport closed",
"connection closed",
"broken pipe",
"end of file",
"eof",
)
SAFE_ENV_KEYS = (
"GITEA_MCP_PROFILE",
"GITEA_PROFILE_NAME",
"GITEA_SERVICE",
"GITEA_EXECUTION_ROLE",
"GITEA_MCP_CONFIG",
)
def _as_list(value: Any) -> list[str] | None:
if value is None:
return None
if isinstance(value, (list, tuple, set)):
return [str(v) for v in value]
return [str(value)]
def _contains_eof(text: str | None) -> bool:
lowered = (text or "").lower()
return any(pattern in lowered for pattern in EOF_PATTERNS)
def _normalize_probe_source(probe_source: str | None) -> str:
raw = (probe_source or PROBE_SOURCE_UNKNOWN).strip().lower()
if raw in VALID_PROBE_SOURCES:
return raw
return PROBE_SOURCE_UNKNOWN
def _safe_env_summary(process: dict[str, Any] | None) -> dict[str, str]:
if not process:
return {}
env = process.get("env") or process.get("environment") or {}
if not isinstance(env, dict):
return {}
return {
key: str(env[key])
for key in SAFE_ENV_KEYS
if key in env and env[key] not in (None, "")
}
def classify_namespace_probe(
namespace: str,
*,
required_tool: str | None = None,
registered_tools: list[str] | tuple[str, ...] | set[str] | None = None,
probe_result: dict[str, Any] | None = None,
process: dict[str, Any] | None = None,
config_path: str | None = None,
profile: str | None = None,
configured: bool = True,
probe_source: str | None = None,
) -> dict[str, Any]:
"""Classify whether a required tool is callable through a live namespace.
``registered_tools`` is static/server-side evidence. ``probe_result`` is
live invocation evidence. Only ``probe_source=client_namespace`` proves the
IDE-managed path; ``offline_spawn`` is an offline subprocess check only.
"""
ns = (namespace or "").strip()
tool = required_tool or REQUIRED_NAMESPACE_TOOLS.get(ns) or "gitea_whoami"
source = _normalize_probe_source(probe_source)
registered_list = _as_list(registered_tools)
registered = None if registered_list is None else tool in registered_list
probe = probe_result or {}
probe_success = bool(probe.get("success"))
error_message = str(
probe.get("error")
or probe.get("message")
or probe.get("stderr")
or probe.get("exception")
or ""
)
error_type = str(probe.get("error_type") or "").strip()
if not error_type and error_message:
if _contains_eof(error_message):
error_type = "namespace_eof"
elif "timeout" in error_message.lower():
error_type = "namespace_timeout"
else:
error_type = "namespace_call_failed"
if not configured:
error_type = "namespace_not_configured"
elif registered is False:
error_type = "tool_missing"
elif not probe_result:
error_type = "live_probe_missing"
elif not probe_success and not error_type:
error_type = "namespace_call_failed"
callable_live = bool(configured and probe_result and probe_success)
# Probe-path health (spawn or client). IDE-proven only for client path.
healthy = bool(configured and registered is not False and callable_live)
ide_namespace_proven = bool(healthy and source == PROBE_SOURCE_CLIENT)
process_pid = process.get("pid") if isinstance(process, dict) else None
profile_name = profile or (
process.get("profile") if isinstance(process, dict) else None
)
env_summary = _safe_env_summary(process)
reasons: list[str] = []
if not configured:
reasons.append(f"MCP namespace '{ns}' is not configured.")
if registered is False:
reasons.append(
f"Required tool '{tool}' is not registered in namespace '{ns}'."
)
if error_type == "live_probe_missing":
reasons.append(
f"No live client invocation proof was supplied for '{ns}.{tool}'."
)
elif error_type == "namespace_eof":
reasons.append(
f"Live MCP namespace '{ns}' returned EOF while invoking '{tool}'."
)
elif error_type == "namespace_timeout":
reasons.append(
f"Live MCP namespace '{ns}' timed out while invoking '{tool}'."
)
elif error_type == "namespace_call_failed":
reasons.append(
f"Live MCP namespace '{ns}' failed while invoking '{tool}'."
)
if source == PROBE_SOURCE_OFFLINE:
reasons.append(
"Probe source is offline_spawn (subprocess JSON-RPC); this does "
"not prove the IDE-managed MCP namespace is healthy."
)
elif source == PROBE_SOURCE_UNKNOWN and probe_result:
reasons.append(
"Probe source unspecified; treat as not IDE-namespace proof unless "
"re-supplied with probe_source='client_namespace'."
)
remediation = []
if not healthy or not ide_namespace_proven:
remediation.append(
"Reconnect the IDE MCP client namespace (client reconnect / "
f"relaunch), then invoke '{tool}' through namespace '{ns}' and "
"record the result with probe_source='client_namespace'."
)
remediation.append(
"Do not treat offline subprocess probes (test_mcp_conn.py) or "
"shell kill/PID respawn as proof the IDE namespace is repaired."
)
if process_pid and source != PROBE_SOURCE_OFFLINE:
remediation.append(
f"Diagnostics may include PID {process_pid}; process details "
"are informational only — recovery is client-layer reconnect."
)
else:
remediation.append(
f"IDE-managed namespace '{ns}' can invoke '{tool}' "
f"(probe_source={source})."
)
# Client-namespace broken health blocks review/merge. Offline probes never
# authorize mutations and only block when they report unhealthy (still
# fail-closed for known bad spawn evidence).
blocks = False
if source == PROBE_SOURCE_CLIENT:
blocks = namespace_health_blocks_task("merge_pr", healthy)
elif source == PROBE_SOURCE_OFFLINE:
# Offline never proves IDE health; never unblock. Unhealthy offline
# still surfaces as a soft diagnostic, not a mutation-ledger block.
blocks = False
else:
# Unknown source: only block when evidence is unhealthy (fail closed
# on bad data without treating success as IDE proof).
blocks = namespace_health_blocks_task("merge_pr", healthy)
return {
"success": healthy,
"healthy": healthy,
"namespace": ns,
"required_tool": tool,
"configured": configured,
"registered_tools_checked": registered_list is not None,
"required_tool_registered": registered,
"required_tool_callable": callable_live,
"probe_source": source,
"ide_namespace_proven": ide_namespace_proven,
"error_type": None if healthy else error_type,
"error_message": error_message or None,
"reasons": reasons,
"remediation": remediation,
"diagnostics": {
"namespace": ns,
"required_tool": tool,
"process_pid": process_pid,
"profile": profile_name,
"env": env_summary,
"config_path": config_path,
"probe_source": source,
},
"blocks_merge_workflow": blocks,
}
def namespace_health_blocks_task(task: str, healthy: bool) -> bool:
"""Return whether a broken namespace must block a workflow task."""
if healthy:
return False
return (task or "").strip() in {"merge_pr", "review_pr", "submit_review"}
def required_namespace_for_task(task: str) -> str | None:
"""Map a mutation task to the MCP namespace that must be healthy."""
return TASK_REQUIRED_NAMESPACES.get((task or "").strip())
def mutation_gate_from_session(
task: str,
session_health: dict[str, dict[str, Any]] | None,
) -> list[str]:
"""Fail-closed gate using recorded client-namespace health assessments.
* Missing session entry → no gate (caller has not assessed yet).
* Client-namespace unhealthy / not IDE-proven → block mutation.
* Offline-only session entries never authorize mutations.
"""
ns = required_namespace_for_task(task)
if not ns:
return []
store = session_health or {}
entry = store.get(ns)
if not entry:
return []
source = _normalize_probe_source(entry.get("probe_source"))
if source != PROBE_SOURCE_CLIENT:
return [
f"recorded namespace health for '{ns}' is probe_source={source}, "
"not client_namespace; re-probe through the IDE-managed path "
f"before {(task or 'mutation')}"
]
if entry.get("ide_namespace_proven") and entry.get("healthy"):
return []
if entry.get("blocks_merge_workflow") or not entry.get("healthy"):
detail = entry.get("error_type") or "unhealthy"
return [
f"live MCP namespace '{ns}' is recorded {detail} "
f"(probe_source={source}); repair the IDE namespace before "
f"{(task or 'mutation')} (fail closed, #543)"
]
if not entry.get("ide_namespace_proven"):
return [
f"live MCP namespace '{ns}' is not IDE-proven; supply a "
"client_namespace probe before mutation (fail closed, #543)"
]
return []
+3
View File
@@ -6,6 +6,9 @@ Runs over stdio. All tools authenticate via macOS keychain (git credential fill)
import os import os
import sys import sys
sys.stderr = open("/tmp/mcp_server_stderr.log", "a", buffering=1)
sys.stderr.write(f"\n--- MCP SERVER STARTUP (PID {os.getpid()}) ---\n")
from role_session_router import ( from role_session_router import (
python_bytes_have_conflict_markers, python_bytes_have_conflict_markers,
skip_python_scan_walk_root, skip_python_scan_walk_root,
+3
View File
@@ -30,6 +30,9 @@ DEFAULT_TTL_HOURS = 4.0
KIND_WORKFLOW_LOAD = "review_workflow_load" KIND_WORKFLOW_LOAD = "review_workflow_load"
KIND_DECISION_LOCK = "review_decision_lock" KIND_DECISION_LOCK = "review_decision_lock"
KIND_REVIEW_DRAFT = "review_draft"
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+") _SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
SESSION_PROFILE_LOCK_ENV = "GITEA_SESSION_PROFILE_LOCK" SESSION_PROFILE_LOCK_ENV = "GITEA_SESSION_PROFILE_LOCK"
+110
View File
@@ -7,6 +7,7 @@ after formal approval at the current PR head.
from __future__ import annotations from __future__ import annotations
import re
from datetime import datetime, timezone from datetime import datetime, timezone
from typing import Any from typing import Any
@@ -133,6 +134,115 @@ def is_sanctioned_session_lease(session: dict[str, Any] | None) -> bool:
return False return False
def describe_session_lease_proof(
session: dict[str, Any] | None,
) -> dict[str, Any]:
"""Canonical merge-report lease proof fields (#535).
Surfaces how the in-session lease was established so merge handoffs can
distinguish sanctioned MCP acquire/adopt paths from bare manual seeding.
"""
if not session:
return {
"lease_proof_source": None,
"lease_recovery_reason": None,
"lease_proof_kind": "none",
"lease_proof_sanctioned": False,
"lease_proof_comment_id": None,
"lease_adopted_from_session_id": None,
}
provenance = session.get("lease_provenance") or {}
if not isinstance(provenance, dict):
provenance = {}
source = (provenance.get("source") or "").strip() or None
sanctioned = is_sanctioned_session_lease(session)
reason = provenance.get("adoption_reason")
if isinstance(reason, str):
reason = reason.strip() or None
else:
reason = None
if source == SOURCE_ADOPT and sanctioned:
kind = "sanctioned_adoption"
reason = reason or DEFAULT_ADOPTION_REASON
elif source == SOURCE_ACQUIRE and sanctioned:
kind = "sanctioned_acquire"
elif source == SOURCE_HEARTBEAT and sanctioned:
kind = "sanctioned_heartbeat"
elif source:
kind = "unsanctioned"
else:
# Session present without provenance = classic manual _SESSION_LEASE seed.
kind = "unsanctioned_manual_seed"
comment_id = provenance.get("comment_id")
if comment_id is None:
comment_id = session.get("comment_id")
return {
"lease_proof_source": source,
"lease_recovery_reason": reason,
"lease_proof_kind": kind,
"lease_proof_sanctioned": sanctioned,
"lease_proof_comment_id": comment_id,
"lease_adopted_from_session_id": provenance.get("adopted_from_session_id"),
}
# Phrases that claim non-MCP / fabricated lease proof in handoffs (#535).
_UNSAFE_LEASE_PROOF_CLAIM_RE = re.compile(
r"(?is)\b("
r"manually\s+seeded\s+lease|"
r"manual(?:ly)?\s+seed(?:ed)?\s+(?:lease|proof)|"
r"seeded\s+lease\s+proof|"
r"injected\s+lease|"
r"lease\s+proof\s+injected|"
r"manual\s+_?SESSION_LEASE|"
r"_SESSION_LEASE\s+seed|"
r"unsanctioned\s+lease\s+proof"
r")\b"
)
# Evidence that the report used the sanctioned MCP adoption/acquire path.
_SANCTIONED_LEASE_EVIDENCE_RE = re.compile(
r"(?is)\b("
r"gitea_adopt_merger_pr_lease|"
r"gitea_acquire_reviewer_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_adopt_merger_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_acquire_reviewer_pr_lease|"
r"lease_proof_kind\s*[:=]\s*sanctioned_adoption|"
r"lease_proof_kind\s*[:=]\s*sanctioned_acquire|"
r"adoption_comment_id\s*[:=]|"
r"sanctioned_adoption|"
r"mcp-review-lease-adoption"
r")\b"
)
def assess_manual_lease_proof_handoff(report_text: str) -> dict[str, Any]:
"""Controller audit: block handoffs that claim unsafe lease seeding (#535).
Reports may discuss manual seeding as a blocked/historical anti-pattern only
when they also cite sanctioned MCP adoption/acquire evidence. Bare claims of
manual/seeded/injected lease proof without that evidence fail closed.
"""
text = report_text or ""
if not _UNSAFE_LEASE_PROOF_CLAIM_RE.search(text):
return {"proven": True, "block": False, "reasons": []}
if _SANCTIONED_LEASE_EVIDENCE_RE.search(text):
return {"proven": True, "block": False, "reasons": []}
return {
"proven": False,
"block": True,
"reasons": [
"report claims manual/seeded/injected/unsanctioned lease proof without "
"sanctioned MCP adoption evidence (use gitea_adopt_merger_pr_lease / "
"gitea_acquire_reviewer_pr_lease and cite lease_proof_source; #535)"
],
}
def assess_adopt_merger_lease( def assess_adopt_merger_lease(
comments: list[dict], comments: list[dict],
*, *,
+222
View File
@@ -0,0 +1,222 @@
"""Canonical post-merge (moot) validation outcome and wording gate (#529).
When a PR is already merged or closed *before* a reviewer submits their
verdict, an ordinary "approve" is misleading: the review never gated the
merge, so a normal active approval overstates controller confidence. The
sanctioned outcome for that case is a **post-merge moot validation** — the
reviewer records what validation found, but classifies it as moot rather than
an active approval.
This module defines the four canonical validation distinctions #529 requires
and enforces the post-merge case:
- ``CLEAN_PASS_OUTCOME`` — clean validation pass on an open, reviewable PR.
- ``BASELINE_ACCEPTED_OUTCOME`` — validation pass with a baseline-proven
unrelated failure (proof handled by :mod:`premerge_baseline_proof`).
- ``BLOCKED_OUTCOME`` — validation blocked by an unresolved failure.
- ``POST_MERGE_MOOT_OUTCOME`` — the PR was already merged/closed before review
submission; validation is recorded as post-merge moot, not an active
approval.
The gate blocks two mistakes:
1. Claiming an *active approval* on a PR that was already merged/closed before
review submission (must be recorded as post-merge moot validation instead).
2. Claiming post-merge moot validation without proof the PR is actually
merged/closed (a merged-state field plus a merge commit SHA).
Each outcome maps to a durable ``validation:*`` process-state label so PRs and
issues carry the distinction (#529 acceptance criterion).
"""
from __future__ import annotations
import re
from typing import Any
CLEAN_PASS_OUTCOME = "clean validation pass"
BASELINE_ACCEPTED_OUTCOME = "validation pass with baseline-proven unrelated failure"
BLOCKED_OUTCOME = "validation blocked by unresolved failure"
POST_MERGE_MOOT_OUTCOME = "post-merge moot validation"
# Canonical validation status label a reviewer writes in the report body for the
# post-merge case; kept in sync with validation_status_vocabulary.
POST_MERGE_MOOT_STATUS = "post-merge moot validation"
# Durable process-state labels (registered in issue_workflow_labels).
LABEL_CLEAN_PASS = "validation:clean-pass"
LABEL_BASELINE_ACCEPTED = "validation:baseline-accepted"
LABEL_BLOCKED = "validation:blocked"
LABEL_POST_MERGE_MOOT = "validation:post-merge-moot"
_OUTCOME_TO_LABEL: dict[str, str] = {
CLEAN_PASS_OUTCOME: LABEL_CLEAN_PASS,
BASELINE_ACCEPTED_OUTCOME: LABEL_BASELINE_ACCEPTED,
BLOCKED_OUTCOME: LABEL_BLOCKED,
POST_MERGE_MOOT_OUTCOME: LABEL_POST_MERGE_MOOT,
}
# The PR was already merged/closed before the review was submitted.
_MERGED_BEFORE_REVIEW_RE = re.compile(
r"(?:already[- ]merged"
r"|merged\s+before\s+review"
r"|closed\s+before\s+review"
r"|pr\s+(?:is|was)\s+(?:already\s+)?(?:merged|closed)"
r"|pr\s+state\s*[:=]\s*(?:merged|closed)"
r"|merged\s*/\s*closed)",
re.IGNORECASE,
)
# An active approval verdict is being claimed.
_ACTIVE_APPROVAL_RE = re.compile(
r"(?:review\s+decision\s*[:=]\s*approve"
r"|submitted\s+['\"]?approve['\"]?"
r"|active\s+approval"
r"|approving\s+the\s+pr"
r"|posting\s+an?\s+approval)",
re.IGNORECASE,
)
# The sanctioned post-merge moot validation wording.
_POST_MERGE_MOOT_RE = re.compile(
r"post[- ]merge\s+(?:moot\s+)?validation"
r"|post[- ]merge\s+moot"
r"|moot\s+validation",
re.IGNORECASE,
)
# Proof the PR is genuinely merged/closed.
_MERGED_STATE_PROOF_RE = re.compile(
r"(?:pr\s+state\s*[:=]\s*(?:merged|closed)"
r"|merged_at\s*[:=]\s*\S"
r"|closed_at\s*[:=]\s*\S"
r"|state\s*[:=]\s*(?:merged|closed))",
re.IGNORECASE,
)
_MERGE_COMMIT_SHA_RE = re.compile(
r"merge\s+commit(?:\s+sha)?\s*[:=]\s*[0-9a-f]{7,40}",
re.IGNORECASE,
)
POST_MERGE_VALIDATION_TEMPLATE = (
"Validation status: post-merge moot validation\n"
"PR state: merged\n"
"Merge commit sha: <40-hex merge commit>\n"
"Validation finding: <what the validation run showed, for the record>\n"
"Note: PR merged/closed before review submission; recorded as post-merge "
"moot validation, not an active approval."
)
def process_state_label(outcome: str) -> str | None:
"""Return the durable ``validation:*`` label for a canonical outcome."""
return _OUTCOME_TO_LABEL.get(outcome)
def assess_post_merge_validation(
report_text: str,
*,
pr_merged_or_closed: bool | None = None,
) -> dict[str, Any]:
"""Assess post-merge moot validation wording and proof (#529).
Args:
report_text: The reviewer final report text.
pr_merged_or_closed: Optional structured signal that the PR is already
merged/closed. When ``True`` it forces the merged-before-review
path even if the report text omits the phrasing; proof fields are
still required in the report body for durability.
Returns a dict with ``proven``, ``block``, ``outcome``,
``process_state_label``, ``reasons``, ``skipped`` and ``safe_next_action``.
Only blocks when the report either claims an active approval on a
merged/closed PR, or claims post-merge moot validation without proof.
"""
text = report_text or ""
merged_signal = bool(_MERGED_BEFORE_REVIEW_RE.search(text)) or bool(
pr_merged_or_closed
)
active_approval = bool(_ACTIVE_APPROVAL_RE.search(text))
moot_wording = bool(_POST_MERGE_MOOT_RE.search(text))
# Nothing about merged-state or moot validation — not applicable here.
if not merged_signal and not moot_wording:
return {
"proven": True,
"block": False,
"outcome": None,
"process_state_label": None,
"reasons": [],
"skipped": True,
"safe_next_action": "",
}
# An active approval on a PR already merged/closed before review, without
# the sanctioned moot wording, overstates confidence.
if merged_signal and active_approval and not moot_wording:
return {
"proven": False,
"block": True,
"outcome": POST_MERGE_MOOT_OUTCOME,
"process_state_label": LABEL_POST_MERGE_MOOT,
"reasons": [
"PR was already merged/closed before review submission; an "
"active approval overstates confidence — record 'post-merge "
"moot validation' instead of a normal approval"
],
"skipped": False,
"safe_next_action": (
"classify the outcome as post-merge moot validation (not an "
"active approval); cite PR state merged/closed and the merge "
"commit SHA. Template:\n" + POST_MERGE_VALIDATION_TEMPLATE
),
}
# Post-merge moot validation claimed — require merged-state + commit proof.
if moot_wording:
reasons: list[str] = []
if not _MERGED_STATE_PROOF_RE.search(text):
reasons.append(
"post-merge moot validation claimed without merged/closed state "
"proof (state: merged/closed, or merged_at/closed_at)"
)
if not _MERGE_COMMIT_SHA_RE.search(text):
reasons.append(
"post-merge moot validation claimed without a merge commit SHA"
)
if reasons:
return {
"proven": False,
"block": True,
"outcome": POST_MERGE_MOOT_OUTCOME,
"process_state_label": LABEL_POST_MERGE_MOOT,
"reasons": reasons,
"skipped": False,
"safe_next_action": (
"prove the PR is merged/closed: cite PR state and the merge "
"commit SHA. Template:\n" + POST_MERGE_VALIDATION_TEMPLATE
),
}
return {
"proven": True,
"block": False,
"outcome": POST_MERGE_MOOT_OUTCOME,
"process_state_label": LABEL_POST_MERGE_MOOT,
"reasons": [],
"skipped": False,
"safe_next_action": "",
}
# Merged signal present but no approval claim and no moot wording yet —
# guide toward the moot outcome without blocking.
return {
"proven": True,
"block": False,
"outcome": POST_MERGE_MOOT_OUTCOME,
"process_state_label": LABEL_POST_MERGE_MOOT,
"reasons": [],
"skipped": False,
"safe_next_action": (
"PR appears merged/closed; if submitting a verdict, record "
"post-merge moot validation rather than an active approval"
),
}
+26 -2
View File
@@ -16,11 +16,35 @@ or the local remote URL is unavailable (best-effort corroboration only).
from __future__ import annotations from __future__ import annotations
import re
REMEDIATION = ( REMEDIATION = (
"Pass explicit org= and repo= matching the local git remote, " "Pass explicit org= and repo= matching the local git remote on tools that "
"e.g. org=Scaled-Tech-Consulting repo=Gitea-Tools." "accept those parameters (including mcp_get_control_plane_guide), "
"e.g. org=Scaled-Tech-Consulting repo=Gitea-Tools. "
"Do not pass org/repo to tools whose schema does not list them."
) )
# https://host/org/repo.git or git@host:org/repo.git
_REMOTE_URL_SLUG_RE = re.compile(
r"(?:[:/])(?P<org>[^/]+)/(?P<repo>[^/]+?)(?:\.git)?/*$"
)
def parse_org_repo_from_remote_url(remote_url: str | None) -> tuple[str, str] | None:
"""Best-effort parse of org/repo from a git remote URL."""
url = (remote_url or "").strip()
if not url:
return None
match = _REMOTE_URL_SLUG_RE.search(url)
if not match:
return None
org = (match.group("org") or "").strip()
repo = (match.group("repo") or "").strip()
if not org or not repo:
return None
return org, repo
def assess_remote_repo_match( def assess_remote_repo_match(
*, *,
+25 -8
View File
@@ -93,8 +93,16 @@ def assess_workflow_blockers(
capability_blocked: bool = False, capability_blocked: bool = False,
mcp_reconnect_failed: bool = False, mcp_reconnect_failed: bool = False,
stale_capability_state: bool = False, stale_capability_state: bool = False,
live_namespace_broken: bool = False,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""Return hard blockers that forbid all PR queue work (#290 AC3).""" """Return hard blockers that forbid all PR queue work (#290 AC3).
``live_namespace_broken`` fails the merge/review path closed when the live
MCP namespace call path is unusable (e.g. ``client is closing: EOF``) even
though the tool is registered in FastMCP (#543 AC5). Feed it the
``blocks_merge_workflow`` verdict from
``mcp_namespace_health.classify_namespace_probe``.
"""
reasons: list[str] = [] reasons: list[str] = []
if infra_stop: if infra_stop:
reasons.append("infra_stop is active; PR selection/review/merge is forbidden") reasons.append("infra_stop is active; PR selection/review/merge is forbidden")
@@ -104,6 +112,12 @@ def assess_workflow_blockers(
reasons.append("MCP reconnect failed; stale session state cannot be reused") reasons.append("MCP reconnect failed; stale session state cannot be reused")
if stale_capability_state: if stale_capability_state:
reasons.append("stale MCP capability state detected after reconnect failure") reasons.append("stale MCP capability state detected after reconnect failure")
if live_namespace_broken:
reasons.append(
"live MCP namespace call path is broken (registered in FastMCP but "
"not callable through the namespace); repair the namespace before "
"review/merge"
)
return { return {
"block": bool(reasons), "block": bool(reasons),
"reasons": reasons, "reasons": reasons,
@@ -118,16 +132,19 @@ def assess_state_advancement(
state_completion: dict[str, bool] | None, state_completion: dict[str, bool] | None,
*, *,
target_state: str, target_state: str,
infra_stop: bool = False, **blocker_kwargs,
capability_blocked: bool = False,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""Fail closed when *target_state* is requested before upstream gates pass.""" """Fail closed when *target_state* is requested before upstream gates pass.
Forwards every blocker flag (``infra_stop``, ``capability_blocked``,
``mcp_reconnect_failed``, ``stale_capability_state``,
``live_namespace_broken``) to :func:`assess_workflow_blockers` so
``can_approve``/``can_merge`` honor the full blocker set, not just
infra_stop/capability_blocked (#543 AC5).
"""
completion = dict(state_completion or {}) completion = dict(state_completion or {})
target = _clean(target_state).upper() target = _clean(target_state).upper()
blockers = assess_workflow_blockers( blockers = assess_workflow_blockers(**blocker_kwargs)
infra_stop=infra_stop,
capability_blocked=capability_blocked,
)
reasons = list(blockers["reasons"]) reasons = list(blockers["reasons"])
try: try:
+167
View File
@@ -4842,6 +4842,22 @@ RECONCILER_CLOSE_REPORT_FIELDS = (
"no review/merge confirmation", "no review/merge confirmation",
) )
RECONCILER_SUPERSESSION_REPORT_FIELDS = (
"identity/profile",
"supersession close capability proof",
"target PR live state",
"target PR independent-work proof",
"superseding PR merged state",
"superseding merge commit SHA",
"target branch SHA",
"superseding ancestry proof",
"canonical close comment",
"linked issue status",
"PR close result",
"issue close result",
"no review/merge confirmation",
)
def assess_reconciler_close_gate( def assess_reconciler_close_gate(
*, *,
@@ -4988,6 +5004,157 @@ def assess_reconciler_close_gate(
} }
def assess_reconciler_supersession_close_gate(
*,
target_pr_number: int | None,
target_pr_state: str | None,
target_pr_mergeable: bool | None,
target_independently_required: bool | None,
superseding_pr_number: int | None,
superseding_pr_state: str | None,
superseding_pr_merged: bool,
superseding_merge_commit_sha: str | None,
superseding_head_sha: str | None,
target_branch: str | None,
target_branch_sha: str | None,
superseding_head_is_ancestor_of_target: bool | None,
canonical_comment_valid: bool,
canonical_comment_mentions_superseding_pr: bool,
canonical_comment_mentions_merge_commit: bool,
close_pr_capability: bool,
close_issue_capability: bool = False,
target_issue_state: str | None = None,
issue_satisfied_by_superseding: bool = False,
) -> dict:
"""Gate reconciler closure of PRs/issues superseded by a merged PR.
This is stricter than already-landed cleanup: the target PR may be closed
only after a named superseding PR is live-verified merged, its head is on a
freshly fetched target branch, the target PR is proven not independently
required, and a canonical close comment cites the superseding PR and merge
commit.
"""
reasons: list[str] = []
if not isinstance(target_pr_number, int) or target_pr_number <= 0:
reasons.append("target PR number missing or invalid (#525)")
if not isinstance(superseding_pr_number, int) or superseding_pr_number <= 0:
reasons.append("superseding PR number missing or invalid (#525)")
if target_pr_number and superseding_pr_number and (
target_pr_number == superseding_pr_number
):
reasons.append("target PR and superseding PR must differ (#525)")
if (target_pr_state or "").strip().lower() != "open":
reasons.append("target PR must be live-verified open before close (#525)")
if target_pr_mergeable is True:
reasons.append(
"target PR is still mergeable; prove it is superseded before close (#525)"
)
if target_independently_required is not False:
reasons.append(
"target PR independent-work proof missing; close requires explicit "
"proof that no required work remains (#525)"
)
if (superseding_pr_state or "").strip().lower() != "closed":
reasons.append("superseding PR must be live-verified closed (#525)")
if superseding_pr_merged is not True:
reasons.append("superseding PR must be live-verified merged (#525)")
if not _FULL_SHA.match((superseding_merge_commit_sha or "").strip()):
reasons.append("superseding merge commit SHA missing or invalid (#525)")
if not _FULL_SHA.match((superseding_head_sha or "").strip()):
reasons.append("superseding head SHA missing or invalid (#525)")
if not (target_branch or "").strip():
reasons.append("target branch missing (#525)")
if not _FULL_SHA.match((target_branch_sha or "").strip()):
reasons.append("target branch SHA missing or invalid (#525)")
if superseding_head_is_ancestor_of_target is None:
reasons.append("superseding ancestry proof not checked (#525)")
if not canonical_comment_valid:
reasons.append("canonical close comment failed validation (#525)")
if not canonical_comment_mentions_superseding_pr:
reasons.append("canonical comment must cite superseding PR (#525)")
if not canonical_comment_mentions_merge_commit:
reasons.append("canonical comment must cite merge commit SHA (#525)")
never_allowed = {
"review_allowed": False,
"approve_allowed": False,
"request_changes_allowed": False,
"merge_allowed": False,
}
if reasons:
return {
"outcome": "GATE_NOT_PROVEN",
"pr_close_allowed": False,
"issue_close_allowed": False,
"reasons": reasons,
"required_report_fields": RECONCILER_SUPERSESSION_REPORT_FIELDS,
"safe_next_action": (
"prove superseding PR state, target branch ancestry, target "
"supersession, and canonical comment before closing"
),
**never_allowed,
}
if superseding_head_is_ancestor_of_target is False:
return {
"outcome": "SUPERSEDING_PR_NOT_ON_TARGET",
"pr_close_allowed": False,
"issue_close_allowed": False,
"reasons": [
f"superseding PR #{superseding_pr_number} head is not an "
f"ancestor of {target_branch}; supersession close denied (#525)"
],
"required_report_fields": RECONCILER_SUPERSESSION_REPORT_FIELDS,
"safe_next_action": "re-fetch target branch and re-check ancestry",
**never_allowed,
}
if close_pr_capability is not True:
return {
"outcome": "RECOVERY_HANDOFF_REQUIRED",
"pr_close_allowed": False,
"issue_close_allowed": False,
"reasons": [
"exact gitea.pr.close capability not proven; produce a "
"recovery handoff instead of closing (#525)"
],
"required_report_fields": RECONCILER_SUPERSESSION_REPORT_FIELDS,
"safe_next_action": (
"launch a reconciler profile with gitea.pr.close and replay "
"the live-state proof"
),
**never_allowed,
}
issue_close_allowed = (
(target_issue_state or "").strip().lower() == "open"
and issue_satisfied_by_superseding is True
and close_issue_capability is True
)
issue_reasons = []
if (target_issue_state or "").strip().lower() == "closed":
issue_reasons.append("linked issue already closed; no issue close attempted (#525)")
elif target_issue_state and not issue_close_allowed:
issue_reasons.append(
"issue close requires an open issue satisfied by the superseding "
"merged PR and exact gitea.issue.close capability (#525)"
)
return {
"outcome": "SUPERSESSION_CLOSE_ALLOWED",
"pr_close_allowed": True,
"issue_close_allowed": issue_close_allowed,
"reasons": issue_reasons,
"required_report_fields": RECONCILER_SUPERSESSION_REPORT_FIELDS,
"safe_next_action": (
"post the canonical comment, close the superseded PR, and close "
"the linked issue only when issue_close_allowed is true"
),
**never_allowed,
}
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Identity disclosure (#305) # Identity disclosure (#305)
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
+238 -1
View File
@@ -522,4 +522,241 @@ def assess_lease_inventory(
"active_review_leases": active, "active_review_leases": active,
"stale_review_leases": stale, "stale_review_leases": stale,
"reclaimable_review_leases": reclaimable, "reclaimable_review_leases": reclaimable,
} }
# Canonical next-action vocabulary for reviewer lease handoff (#599).
NEXT_ACTION_ACQUIRE = "acquire"
NEXT_ACTION_WAIT = "wait"
NEXT_ACTION_RESUME_EXACT_OWNER_SESSION = "resume_exact_owner_session"
NEXT_ACTION_RELEASE_EXPIRED_LEASE = "release_expired_lease"
NEXT_ACTION_OPERATOR_AUTHORIZED_CLEANUP = "operator_authorized_cleanup"
NEXT_ACTION_REPAIR_WORKTREE_BINDING = "repair_worktree_binding"
_HANDOFF_CLASSIFICATIONS = frozenset({
"no_lease",
"own_active",
"own_expired",
"foreign_active",
"foreign_reclaimable",
"foreign_expired",
"instructed_lease_missing_with_replacement",
"worktree_binding_mismatch",
})
def _norm_path(value: str | None) -> str:
text = (value or "").strip()
if not text:
return ""
return os.path.normpath(text.rstrip("/"))
def diagnose_reviewer_pr_lease_handoff(
comments: list[dict],
*,
pr_number: int,
current_session_id: str | None,
current_reviewer_identity: str | None,
proposed_worktree: str | None = None,
env_bound_worktree: str | None = None,
instructed_session_id: str | None = None,
instructed_comment_id: int | None = None,
now: datetime | None = None,
) -> dict[str, Any]:
"""Classify open-PR reviewer lease handoff and emit a canonical next action (#599).
Read-only diagnosis. Never steals, releases, or adopts a foreign lease.
Fail-closed acquisition rules for active foreign leases remain intact.
Returns a structured diagnosis with:
- classification
- next_action (one of wait / resume_exact_owner_session /
release_expired_lease / operator_authorized_cleanup /
repair_worktree_binding / acquire)
- active_lease identity fields when present
- worktree_binding match result
- instructed-lease mismatch flags
"""
now = now or datetime.now(timezone.utc)
session_id = (current_session_id or "").strip()
identity = (current_reviewer_identity or "").strip()
instructed_sid = (instructed_session_id or "").strip()
reasons: list[str] = []
active = find_active_reviewer_lease(comments, pr_number=pr_number, now=now)
session_lease = get_session_lease()
# Worktree binding: env-bound vs proposed vs active lease worktree.
env_wt = _norm_path(env_bound_worktree)
prop_wt = _norm_path(proposed_worktree)
lease_wt = _norm_path((active or {}).get("worktree") if active else None)
binding_mismatch = False
binding_details: dict[str, Any] = {
"env_bound_worktree": env_bound_worktree or None,
"proposed_worktree": proposed_worktree or None,
"lease_worktree": (active or {}).get("worktree") if active else None,
"match": True,
}
paths = [p for p in (env_wt, prop_wt, lease_wt) if p]
if len(paths) >= 2 and len(set(paths)) > 1:
binding_mismatch = True
binding_details["match"] = False
reasons.append(
"worktree binding mismatch: env/proposed/lease worktree paths disagree"
)
# Instructed lease gone while a different lease is active (PR #592-style).
instructed_missing_with_replacement = False
if instructed_sid or instructed_comment_id is not None:
if not active:
reasons.append(
"instructed lease is gone and no active replacement lease remains"
)
else:
owner = (active.get("session_id") or "").strip()
cid = active.get("comment_id")
sid_mismatch = bool(instructed_sid and owner and owner != instructed_sid)
cid_mismatch = (
instructed_comment_id is not None
and cid is not None
and int(cid) != int(instructed_comment_id)
)
if sid_mismatch or cid_mismatch:
instructed_missing_with_replacement = True
reasons.append(
"instructed lease is gone; a different active lease replaced it "
f"(active session_id={owner}, comment_id={cid})"
)
# Classification + next_action.
classification = "no_lease"
next_action = NEXT_ACTION_ACQUIRE
if active:
owner = (active.get("session_id") or "").strip()
freshness = active.get("freshness") or classify_lease_freshness(
active, now=now
)
owner_identity = (active.get("reviewer_identity") or "").strip()
is_own = bool(session_id and owner and owner == session_id)
# Same identity alone is NOT ownership for resume; session_id must match.
same_identity = bool(
identity and owner_identity and identity == owner_identity
)
if is_own and freshness in {"active", "stale_warning"}:
classification = "own_active"
next_action = NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
elif is_own and freshness in {"reclaimable", "expired"}:
classification = "own_expired"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"own lease freshness is '{freshness}'; release via "
"gitea_release_reviewer_pr_lease then re-acquire"
)
elif not is_own and freshness in {"active", "stale_warning"}:
classification = "foreign_active"
next_action = NEXT_ACTION_WAIT
reasons.append(
f"foreign active reviewer lease (session_id={owner}, "
f"phase={active.get('phase')}, freshness={freshness}); "
"do not submit; do not steal"
)
if same_identity:
reasons.append(
"lease identity matches current reviewer but session_id differs; "
"resume only from the exact owner session_id or wait"
)
elif not is_own and freshness == "reclaimable":
classification = "foreign_reclaimable"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"foreign reclaimable lease (session_id={owner}); clear only via "
"sanctioned gitea_release_reviewer_pr_lease when reclaimable"
)
elif not is_own and freshness == "expired":
classification = "foreign_expired"
next_action = NEXT_ACTION_RELEASE_EXPIRED_LEASE
reasons.append(
f"foreign expired lease (session_id={owner}); use sanctioned release"
)
else:
classification = "foreign_active"
next_action = NEXT_ACTION_WAIT
reasons.append(
f"unclassified active lease state (session_id={owner}, "
f"freshness={freshness}); wait fail-closed"
)
if instructed_missing_with_replacement and classification.startswith(
"foreign"
):
classification = "instructed_lease_missing_with_replacement"
# Foreign active still means wait; reclaimable still release.
if next_action == NEXT_ACTION_WAIT:
reasons.append(
"replacement foreign lease is active — wait; "
"operator_authorized_cleanup only with explicit operator authority"
)
else:
classification = "no_lease"
next_action = NEXT_ACTION_ACQUIRE
reasons.append("no active reviewer lease; acquire via gitea_acquire_reviewer_pr_lease")
# Binding mismatch is a first-class blocker before submit, but does not
# erase foreign-lease wait/release guidance. Override next_action only when
# the session would otherwise be free to acquire or resume (submit path).
if binding_mismatch:
if next_action in {
NEXT_ACTION_ACQUIRE,
NEXT_ACTION_RESUME_EXACT_OWNER_SESSION,
}:
classification = "worktree_binding_mismatch"
next_action = NEXT_ACTION_REPAIR_WORKTREE_BINDING
else:
reasons.append(
"also repair worktree binding before submit "
f"(next_action remains {next_action})"
)
lease_summary = None
if active:
lease_summary = {
"comment_id": active.get("comment_id"),
"session_id": active.get("session_id"),
"phase": active.get("phase"),
"candidate_head": active.get("candidate_head"),
"expires_at": active.get("expires_at"),
"last_activity": active.get("last_activity"),
"freshness": active.get("freshness")
or classify_lease_freshness(active, now=now),
"reviewer_identity": active.get("reviewer_identity"),
"profile": active.get("profile"),
"worktree": active.get("worktree"),
"blocker": active.get("blocker"),
}
return {
"pr_number": pr_number,
"classification": classification,
"next_action": next_action,
"active_lease": lease_summary,
"session_lease": session_lease,
"worktree_binding": binding_details,
"instructed_session_id": instructed_session_id,
"instructed_comment_id": instructed_comment_id,
"instructed_lease_missing_with_replacement": instructed_missing_with_replacement,
"mutation_allowed": (
next_action == NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
and not binding_mismatch
and bool(session_lease)
),
"reasons": reasons,
"forbidden": [
"manual lock deletion",
"raw API bypass",
"mtime manipulation",
"direct _SESSION_LEASE seeding",
"silent foreign lease steal",
],
}
+31 -2
View File
@@ -55,7 +55,6 @@ def skip_python_scan_walk_root(project_root: str, walk_root: str) -> bool:
REVIEWER_TASKS = frozenset({ REVIEWER_TASKS = frozenset({
"review_pr", "review_pr",
"merge_pr",
"blind_pr_queue_review", "blind_pr_queue_review",
"pr_queue_cleanup", "pr_queue_cleanup",
"pr-queue-cleanup", "pr-queue-cleanup",
@@ -63,6 +62,10 @@ REVIEWER_TASKS = frozenset({
"approve_pr", "approve_pr",
}) })
MERGER_TASKS = frozenset({
"merge_pr",
})
AUTHOR_TASKS = frozenset({ AUTHOR_TASKS = frozenset({
"create_issue", "create_issue",
"comment_issue", "comment_issue",
@@ -98,7 +101,7 @@ TASK_REQUIRED_ROLE = {
"address_pr_change_requests": "author", "address_pr_change_requests": "author",
"delete_branch": "author", "delete_branch": "author",
"review_pr": "reviewer", "review_pr": "reviewer",
"merge_pr": "reviewer", "merge_pr": "merger",
"blind_pr_queue_review": "reviewer", "blind_pr_queue_review": "reviewer",
"pr_queue_cleanup": "reviewer", "pr_queue_cleanup": "reviewer",
"pr-queue-cleanup": "reviewer", "pr-queue-cleanup": "reviewer",
@@ -114,6 +117,9 @@ TASK_REQUIRED_ROLE = {
# #309: reconciler tasks close already-landed PRs/issues only. # #309: reconciler tasks close already-landed PRs/issues only.
"reconcile_close_landed_pr": "reconciler", "reconcile_close_landed_pr": "reconciler",
"reconcile_close_landed_issue": "reconciler", "reconcile_close_landed_issue": "reconciler",
"reconcile_close_superseded_pr": "reconciler",
"reconcile_close_satisfied_issue": "reconciler",
"reconcile_create_followup_issue": "reconciler",
} }
WRONG_ROLE_REVIEWER_MSG = ( WRONG_ROLE_REVIEWER_MSG = (
@@ -125,6 +131,10 @@ WRONG_ROLE_RECONCILER_MSG = (
"MCP namespace/profile with exact close capability." "MCP namespace/profile with exact close capability."
) )
WRONG_ROLE_MERGER_MSG = (
"Wrong role/session for merger task. Launch merger MCP namespace."
)
_session_last_route: dict | None = None _session_last_route: dict | None = None
@@ -240,6 +250,25 @@ def route_task_session(
_record_route(result) _record_route(result)
return result return result
if required_role == "merger":
result = {
"task_type": task_type,
"required_role": required_role,
"active_role": active_role_kind,
"active_profile": active_profile,
"route_result": ROUTE_WRONG_ROLE,
"downstream_allowed": False,
"reasons": [
WRONG_ROLE_MERGER_MSG,
"Merger tasks cannot run in author or reviewer sessions.",
],
"message": WRONG_ROLE_MERGER_MSG,
"runtime_switching_supported": runtime_switching_supported,
"profile_switch_blocked": not runtime_switching_supported,
}
_record_route(result)
return result
if required_role == "author": if required_role == "author":
route = ROUTE_TO_AUTHOR route = ROUTE_TO_AUTHOR
message = ( message = (
@@ -344,6 +344,41 @@ If edits are needed, make the smallest correction necessary and report the corre
Do not silently change requested meaning. Do not silently change requested meaning.
## 14a. Enforced content preflight (#582)
`gitea_create_issue` runs a **content preflight gate** before duplicate
search or creation. It fails closed (returns `BLOCKED + DIAGNOSE`, no issue
created) when the title/body is a *vague reference* to out-of-band draft
content the session cannot prove it holds, and no durable source pointer is
present.
An LLM must never fabricate issue content from stale chat memory. If asked to
create "the drafted issue" / "the prepared issue" / "the issue we discussed"
/ "the previous draft" without the full content in the current context or a
durable source pointer, stop and return `BLOCKED + DIAGNOSE` naming the exact
vague/missing fields.
A **durable source pointer** is one of: an existing issue/PR reference
(`#582`), a comment id (`comment 8155`), a checked-in file path
(`task_capability_map.py`), a URL, or a scratchpad path. When a pointer is
present, retrieve the real content from it before creating.
The gate does **not** require a non-empty body; explicit title-only creation
stays allowed. It only blocks placeholder references. An operator may bypass
it with `allow_incomplete_content=True` (report the override).
**Invalid prompts (must block):**
* "Create the drafted issue."
* "File the prepared issue we discussed."
* "Open the previous draft."
**Valid prompts (proceed):**
* Full title + body + acceptance criteria supplied inline.
* "Create the issue drafted in `#582`." (durable pointer → fetch and use it)
* "Create the issue from `scratchpad/issue-draft.md`." (durable file pointer)
## 15. Labels, assignees, and metadata ## 15. Labels, assignees, and metadata
Apply labels, assignees, milestones, or project fields only if: Apply labels, assignees, milestones, or project fields only if:
@@ -826,6 +826,75 @@ 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
## 26A-1. Stale durable review-decision lock cleanup (#594)
After #559, the review-decision lock is durable under
`~/.cache/gitea-tools/session-state/` (for example
`review_decision_lock-prgs-reviewer.json`). That durability can outlive the work
it protects: a terminal `approve` / `request_changes` on a PR that is already
**merged or closed** still hard-stops later unrelated reviews under #332.
**Do not** delete session-state files by hand. Use the canonical MCP path.
### When cleanup is allowed
Use `gitea_cleanup_stale_review_decision_lock` when:
1. `gitea_mark_final_review_decision` / live review is blocked by
`terminal review mutation already consumed ... (#332)`.
2. Live Gitea state for the **last terminal mutation's PR** is **merged** or
**closed** (no same-PR merge sequence remains).
3. Active profile identity matches the durable lock (reviewer profile with
`gitea.pr.review` for `apply=true`).
4. Optional pin: pass `expected_terminal_pr` to the prior terminal PR number
(for example `586` when resuming after a merged #586 lock).
Recommended sequence:
```text
gitea_whoami
gitea_resolve_task_capability(task="cleanup_stale_review_decision_lock")
gitea_cleanup_stale_review_decision_lock(apply=false, remote=..., org=..., repo=...)
# inspect is_moot / cleanup_allowed / last_terminal_pr
gitea_cleanup_stale_review_decision_lock(
apply=true,
expected_terminal_pr=<last_terminal_pr>,
remote=..., org=..., repo=...,
)
gitea_resolve_task_capability(task="review_pr")
gitea_load_review_workflow()
# continue formal mark + submit for the *new* PR
```
Successful `apply=true` clears memory + durable store and returns an audit
record (and posts a durable PR comment on the terminal PR when
`post_audit_comment` is true and comment permission allows).
Same-profile auto-expire: after `gitea_merge_pr` succeeds for the PR that this
profile just approved, the decision lock for that profile is cleared
automatically. Cross-profile locks (for example reviewer approve, merger merge)
still require `gitea_cleanup_stale_review_decision_lock`.
### When cleanup is forbidden
Refuse / fail-closed — keep #332 hard-stop — when:
* the last terminal PR is still **open** (active review or merge sequence may
still apply)
* live PR state cannot be fetched (ambiguous)
* no lock or no terminal live mutation exists
* profile identity does not match the lock
* apply requested without authenticated identity or `gitea.pr.review`
* `expected_terminal_pr` does not match the last terminal PR
Do **not** use cleanup to:
* bypass #332 on an open PR
* replace `gitea_authorize_review_correction` for a mistaken review on a still
active PR (#211)
* auto-approve or auto-merge any PR
* justify `rm` of session-state files
## 26B. Per-PR reviewer lease (#407) ## 26B. Per-PR reviewer lease (#407)
Parallel reviewer sessions are allowed only when each session holds a distinct, Parallel reviewer sessions are allowed only when each session holds a distinct,
@@ -1043,6 +1112,8 @@ Blocked handoffs must say to rerun the full workflow after the blocker clears.
If the blocker is a terminal review mutation already consumed in the current run, the handoff must say that the next run must start from the beginning and must not reuse stale ready/approved state. If the blocker is a terminal review mutation already consumed in the current run, the handoff must say that the next run must start from the beginning and must not reuse stale ready/approved state.
If the referenced terminal PR is already **merged or closed** on live Gitea, the durable #332 decision lock is **moot**. Do **not** delete session-state files by hand. Use `gitea_cleanup_stale_review_decision_lock` (assess with `apply=false`, then `apply=true` with `expected_terminal_pr` set) from the reviewer profile after identity/profile checks (#594). Cleanup is **forbidden** while that PR is still open.
## 30. Final report must be precise ## 30. Final report must be precise
Include: Include:
+440
View File
@@ -0,0 +1,440 @@
"""Stale / moot #332 review-decision lock detection and cleanup policy (#594/#620).
#332 correctly hard-stops a reviewer session after a terminal live review
mutation. After #559 those locks are durable on disk, so they can outlive the
work they protect: when the referenced PR is already merged or closed, no
same-PR merge sequence remains, yet the durable ledger still blocks unrelated
new terminal reviews.
#620 scopes the terminal boundary by **reviewed head SHA**: a prior
REQUEST_CHANGES (or APPROVE) on head A must not block a fresh formal decision
on head B of the **same open PR**. Same-head duplicates remain fail-closed.
Open-PR lock *cleanup* remains forbidden unless the PR is truly merged/closed
(#594); new-head re-review is allowed without deleting the durable lock.
This module is pure policy (no Gitea I/O). The MCP tool
``gitea_cleanup_stale_review_decision_lock`` fetches live PR state, calls these
helpers, and only then clears durable state when cleanup is allowed.
Manual deletion of ``~/.cache/gitea-tools/session-state/*`` is never the
canonical path.
"""
from __future__ import annotations
from datetime import datetime, timezone
from typing import Any
# Keep in sync with gitea_mcp_server._TERMINAL_REVIEW_ACTIONS.
TERMINAL_REVIEW_ACTIONS = frozenset({"approve", "request_changes"})
def normalize_head_sha(value: Any) -> str | None:
"""Normalize a git SHA for equality comparison; None if empty/invalid."""
if value is None:
return None
text = str(value).strip().lower()
if not text:
return None
return text
def heads_equal(a: Any, b: Any) -> bool:
na = normalize_head_sha(a)
nb = normalize_head_sha(b)
if not na or not nb:
return False
return na == nb
def mutation_head_sha(mutation: dict | None, lock: dict | None = None) -> str | None:
"""Head SHA that bounds a live mutation (#620).
Prefers fields recorded on the mutation itself. For *legacy* mutations
that predate head-scoping, falls back to the lock's
``ready_expected_head_sha`` only when that ready binding is for the same
PR number (the frozen durable PR #619-style ledger).
"""
if not isinstance(mutation, dict):
return None
for key in ("head_sha", "expected_head_sha", "reviewed_head_sha"):
head = normalize_head_sha(mutation.get(key))
if head:
return head
if not isinstance(lock, dict):
return None
if lock.get("ready_pr_number") != mutation.get("pr_number"):
return None
return normalize_head_sha(lock.get("ready_expected_head_sha"))
def last_terminal_mutation(lock: dict | None) -> dict | None:
"""Return the last terminal live mutation on *lock*, or None."""
if not lock:
return None
terminals = [
m
for m in (lock.get("live_mutations") or [])
if isinstance(m, dict) and m.get("action") in TERMINAL_REVIEW_ACTIONS
]
return terminals[-1] if terminals else None
def prior_live_mutations_block_boundary(
lock: dict | None,
*,
pr_number: int,
expected_head_sha: str | None,
) -> bool:
"""True when prior live mutations block a new decision for PR+head (#620).
Historical terminals on a **different head of the same PR** do not block.
Any prior mutation on another PR, the same head, or without a comparable
head SHA fails closed (blocks).
Head resolution uses ``mutation_head_sha(m, lock)`` so pre-#620 ledgers
that only stored ``ready_expected_head_sha`` still compare correctly
(PR #619-style).
"""
if not lock:
return False
if lock.get("correction_authorized"):
return False
prior = list(lock.get("live_mutations") or [])
if not prior:
return False
target = normalize_head_sha(expected_head_sha)
for m in prior:
if not isinstance(m, dict):
return True
m_pr = m.get("pr_number")
m_head = mutation_head_sha(m, lock)
if (
m_pr == pr_number
and target
and m_head
and not heads_equal(m_head, target)
):
# Same open PR, different reviewed head — historical boundary only.
continue
return True
return False
def terminal_boundary_allows_fresh_decision(
lock: dict | None,
*,
pr_number: int,
expected_head_sha: str | None,
operation: str,
) -> bool:
"""Whether #332 hard-stop should yield for a new PR+head boundary (#620).
Only for ``mark_ready`` / ``review`` / ``resume`` on the **same PR** when
the requested head differs from the last terminal's head. Merge is never
reopened by head change (approved head merge path is separate).
"""
if operation not in ("mark_ready", "review", "resume"):
return False
if not lock:
return False
if lock.get("correction_authorized"):
return True
last = last_terminal_mutation(lock)
if last is None:
return True
if last.get("pr_number") != pr_number:
return False
locked_head = mutation_head_sha(last, lock)
target = normalize_head_sha(expected_head_sha)
if not locked_head or not target:
return False
return not heads_equal(locked_head, target)
def backfill_terminal_heads_from_ready(lock: dict) -> dict:
"""Stamp head_sha onto legacy terminal mutations before overwriting ready_*.
Called when marking a fresh decision on a new head so historical rows keep
their original boundary after ``ready_expected_head_sha`` advances (#620).
"""
if not isinstance(lock, dict):
return lock
ready_pr = lock.get("ready_pr_number")
ready_head = normalize_head_sha(lock.get("ready_expected_head_sha"))
if ready_pr is None or not ready_head:
return lock
mutations = list(lock.get("live_mutations") or [])
changed = False
for m in mutations:
if not isinstance(m, dict):
continue
if m.get("action") not in TERMINAL_REVIEW_ACTIONS:
continue
if m.get("pr_number") != ready_pr:
continue
if mutation_head_sha(m):
continue
m["head_sha"] = ready_head
changed = True
if changed:
lock["live_mutations"] = mutations
return lock
def classify_pr_live_state(pr_live: dict | None) -> dict[str, Any]:
"""Normalize a Gitea PR payload into merge/closed flags."""
pr = pr_live or {}
merged = bool(pr.get("merged") or pr.get("merged_at"))
state = (pr.get("state") or "").strip().lower() or None
closed = state == "closed"
head = pr.get("head") if isinstance(pr.get("head"), dict) else {}
head_sha = normalize_head_sha(
pr.get("head_sha") or pr.get("head_commit_sha") or head.get("sha")
)
return {
"pr_state": state,
"pr_merged": merged,
"pr_merged_or_closed": merged or closed,
"merge_commit_sha": pr.get("merge_commit_sha")
or pr.get("merged_commit_sha"),
"current_pr_head_sha": head_sha,
}
def lock_summary(lock: dict | None) -> dict[str, Any] | None:
"""LLM-safe summary of a decision lock (no secrets)."""
if lock is None:
return None
last = last_terminal_mutation(lock)
mutations = list(lock.get("live_mutations") or [])
last_head = mutation_head_sha(last, lock) if last else None
return {
"task": lock.get("task"),
"remote": lock.get("remote"),
"org": lock.get("org") or lock.get("ready_org"),
"repo": lock.get("repo") or lock.get("ready_repo"),
"profile_identity": lock.get("profile_identity")
or lock.get("session_profile_lock")
or lock.get("session_profile"),
"session_profile": lock.get("session_profile"),
"ready_pr_number": lock.get("ready_pr_number"),
"ready_action": lock.get("ready_action"),
"ready_expected_head_sha": normalize_head_sha(
lock.get("ready_expected_head_sha")
),
"live_mutations_count": len(mutations),
"last_terminal": (
{
"pr_number": last.get("pr_number"),
"action": last.get("action"),
"review_id": last.get("review_id"),
"head_sha": last_head,
}
if last
else None
),
"locked_head_sha": last_head,
"correction_authorized": bool(lock.get("correction_authorized")),
"updated_at": lock.get("updated_at") or lock.get("recorded_at"),
}
def assess_stale_review_decision_lock(
lock: dict | None,
*,
pr_live: dict | None = None,
pr_lookup_error: str | None = None,
active_profile_identity: str | None = None,
current_pr_head_sha: str | None = None,
) -> dict[str, Any]:
"""Assess whether a durable review-decision lock is stale/moot (#594/#620).
*pr_live* must be the live Gitea payload for the **last terminal
mutation's PR**. When no lock / no terminal mutation exists, cleanup is
not applicable. When the PR is still open (or lookup fails), cleanup is
forbidden (#594). Open PR + different live head reports
``stale_by_head`` / ``fresh_review_on_current_head_allowed`` (#620)
without enabling cleanup.
"""
summary = lock_summary(lock)
result: dict[str, Any] = {
"has_lock": lock is not None,
"lock_summary": summary,
"last_terminal_pr": None,
"last_terminal_action": None,
"locked_head_sha": None,
"current_pr_head_sha": normalize_head_sha(current_pr_head_sha),
"stale_by_head": False,
"fresh_review_on_current_head_allowed": False,
"is_moot": False,
"cleanup_allowed": False,
"profile_match": True,
"pr_state": None,
"pr_merged": False,
"pr_merged_or_closed": False,
"merge_commit_sha": None,
"reasons": [],
"recovery_tool": "gitea_cleanup_stale_review_decision_lock",
}
if lock is None:
result["reasons"].append("no review decision lock present")
return result
# Profile identity gate (caller may re-check with env; pure assess still
# reports mismatch when active identity is supplied).
stored = (
(lock.get("profile_identity") or lock.get("session_profile_lock") or "")
.strip()
)
active = (active_profile_identity or "").strip()
if active and stored and active != stored:
result["profile_match"] = False
result["reasons"].append(
"review decision lock profile identity does not match active "
f"profile '{active}' (fail closed, #594)"
)
return result
last = last_terminal_mutation(lock)
if last is None:
result["reasons"].append(
"review decision lock has no terminal live mutations; nothing to clear"
)
return result
pr_number = last.get("pr_number")
action = last.get("action")
locked_head = mutation_head_sha(last, lock)
result["last_terminal_pr"] = pr_number
result["last_terminal_action"] = action
result["locked_head_sha"] = locked_head
if pr_number is None:
result["reasons"].append(
"last terminal mutation missing pr_number; refuse cleanup (fail closed)"
)
return result
if pr_lookup_error:
result["reasons"].append(
f"could not load live state for PR #{pr_number}: {pr_lookup_error} "
"(fail closed, #594)"
)
return result
if pr_live is None:
result["reasons"].append(
f"live PR state required for terminal PR #{pr_number} before cleanup "
"(fail closed, #594)"
)
return result
live = classify_pr_live_state(pr_live)
current_head = normalize_head_sha(
current_pr_head_sha or live.get("current_pr_head_sha")
)
result.update(
{
"pr_state": live["pr_state"],
"pr_merged": live["pr_merged"],
"pr_merged_or_closed": live["pr_merged_or_closed"],
"merge_commit_sha": live["merge_commit_sha"],
"current_pr_head_sha": current_head,
}
)
if not live["pr_merged_or_closed"]:
stale_by_head = bool(
locked_head and current_head and not heads_equal(locked_head, current_head)
)
result["stale_by_head"] = stale_by_head
# Fresh formal decision is allowed on the new head without cleanup (#620).
result["fresh_review_on_current_head_allowed"] = stale_by_head
if stale_by_head:
result["reasons"].append(
f"terminal {action} on PR #{pr_number} is bound to head "
f"{locked_head[:12]}…; live head is {current_head[:12]}… — "
"fresh formal review on current head is allowed without "
"cleanup (fail closed for cleanup, #620); #332 same-head "
"duplicate protection remains"
)
else:
result["reasons"].append(
f"terminal review mutation on open PR #{pr_number} is still active "
f"({action}); #332 hard-stop remains — cleanup forbidden (#594)"
)
return result
# Merged or closed: lock is moot. Same-PR merge continuation is impossible.
result["is_moot"] = True
result["cleanup_allowed"] = True
result["stale_by_head"] = False
result["fresh_review_on_current_head_allowed"] = False
result["reasons"].append(
f"terminal mutation ({action} on PR #{pr_number}) is moot: PR is "
f"{'merged' if live['pr_merged'] else 'closed'}; canonical cleanup allowed (#594)"
)
return result
def build_cleanup_audit_record(
*,
assessment: dict[str, Any],
actor_username: str | None,
profile_name: str | None,
applied: bool,
) -> dict[str, Any]:
"""Structured durable audit payload for a cleanup attempt."""
last = (assessment.get("lock_summary") or {}).get("last_terminal") or {}
return {
"event": "stale_review_decision_lock_cleanup",
"issue_ref": "#594",
"applied": bool(applied),
"timestamp": datetime.now(timezone.utc).isoformat(),
"actor_username": actor_username,
"profile_name": profile_name,
"last_terminal_pr": assessment.get("last_terminal_pr") or last.get("pr_number"),
"last_terminal_action": assessment.get("last_terminal_action")
or last.get("action"),
"pr_state": assessment.get("pr_state"),
"pr_merged": assessment.get("pr_merged"),
"pr_merged_or_closed": assessment.get("pr_merged_or_closed"),
"merge_commit_sha": assessment.get("merge_commit_sha"),
"prior_live_mutations_count": (
(assessment.get("lock_summary") or {}).get("live_mutations_count")
),
"prior_profile_identity": (
(assessment.get("lock_summary") or {}).get("profile_identity")
),
"cleanup_allowed": assessment.get("cleanup_allowed"),
"is_moot": assessment.get("is_moot"),
"reasons": list(assessment.get("reasons") or []),
}
def format_cleanup_audit_comment(audit: dict[str, Any]) -> str:
"""Markdown body for a durable Gitea audit comment after cleanup."""
status = "APPLIED" if audit.get("applied") else "ASSESSED (not applied)"
lines = [
"## Stale #332 review-decision lock cleanup (#594)",
"",
f"Status: **{status}**",
"",
f"- actor: `{audit.get('actor_username')}`",
f"- profile: `{audit.get('profile_name')}`",
f"- timestamp: `{audit.get('timestamp')}`",
f"- last terminal: `{audit.get('last_terminal_action')}` on "
f"PR #{audit.get('last_terminal_pr')}",
f"- PR state: `{audit.get('pr_state')}` "
f"(merged={audit.get('pr_merged')})",
f"- merge_commit_sha: `{audit.get('merge_commit_sha')}`",
f"- prior live_mutations_count: "
f"`{audit.get('prior_live_mutations_count')}`",
f"- prior profile_identity: `{audit.get('prior_profile_identity')}`",
"",
"Manual deletion of session-state files is **not** the workflow.",
"This path only clears a lock when the referenced PR is merged/closed.",
]
return "\n".join(lines)
+34
View File
@@ -96,6 +96,17 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.approve", "permission": "gitea.pr.approve",
"role": "reviewer", "role": "reviewer",
}, },
# #594: clear durable #332 decision lock only when last terminal PR is
# already merged/closed (moot). Apply path requires reviewer review
# permission; assessment itself uses gitea.read inside the tool.
"cleanup_stale_review_decision_lock": {
"permission": "gitea.pr.review",
"role": "reviewer",
},
"gitea_cleanup_stale_review_decision_lock": {
"permission": "gitea.pr.review",
"role": "reviewer",
},
"delete_branch": { "delete_branch": {
"permission": "gitea.branch.delete", "permission": "gitea.branch.delete",
"role": "author", "role": "author",
@@ -128,6 +139,17 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.create", "permission": "gitea.pr.create",
"role": "author", "role": "author",
}, },
# #600: controller-owned allocator — any authenticated profile may call;
# routing enforces role match to selected work. Uses control-plane DB (#613).
"allocate_next_work": {
"permission": "gitea.read",
"role": "author",
},
"gitea_allocate_next_work": {
"permission": "gitea.read",
"role": "author",
},
"reconcile_landed_pr": { "reconcile_landed_pr": {
"permission": "gitea.read", "permission": "gitea.read",
"role": "author", "role": "author",
@@ -150,6 +172,18 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.issue.close", "permission": "gitea.issue.close",
"role": "reconciler", "role": "reconciler",
}, },
"reconcile_close_superseded_pr": {
"permission": "gitea.pr.close",
"role": "reconciler",
},
"reconcile_close_satisfied_issue": {
"permission": "gitea.issue.close",
"role": "reconciler",
},
"reconcile_create_followup_issue": {
"permission": "gitea.issue.create",
"role": "reconciler",
},
"post_heartbeat": { "post_heartbeat": {
"permission": "gitea.issue.comment", "permission": "gitea.issue.comment",
"role": "author", "role": "author",
+185 -25
View File
@@ -1,20 +1,45 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
"""Live health-check script to verify Gitea MCP namespace connections. """Offline-only MCP namespace spawn probe (NOT IDE-namespace proof).
Spawns the MCP server processes as defined in the IDE's global config, Spawns a *separate* MCP server process from config via subprocess.Popen,
performs the JSON-RPC handshake, and queries the tools list to verify performs the JSON-RPC handshake, verifies the required tool is registered,
that the connection is fully operational and doesn't return EOF. and invokes that tool. Results are classified with
``probe_source=offline_spawn``.
This path is useful for offline launch/registration debugging. It does
**not** prove the IDE-managed MCP client namespace is healthy (#543). For
workflow gates, pass live IDE call evidence with
``probe_source=client_namespace`` to ``gitea_assess_mcp_namespace_health``.
""" """
import argparse
import json import json
import os import os
import subprocess import subprocess
import sys import sys
def run_connection_test(name, config): from mcp_namespace_health import REQUIRED_NAMESPACE_TOOLS, classify_namespace_probe
def _read_json_line(proc):
line = proc.stdout.readline()
if not line:
stderr_content = proc.stderr.read()
return None, stderr_content
return json.loads(line), None
def _write_message(proc, payload):
proc.stdin.write(json.dumps(payload) + "\n")
proc.stdin.flush()
def run_connection_test(name, config, *, required_tool=None, config_path=None):
print(f"Testing MCP connection for '{name}'...") print(f"Testing MCP connection for '{name}'...")
command = config.get("command") command = config.get("command")
args = config.get("args", []) args = config.get("args", [])
env = config.get("env", {}) env = config.get("env", {})
tool_name = required_tool or REQUIRED_NAMESPACE_TOOLS.get(name) or "gitea_whoami"
# Merge current environment # Merge current environment
run_env = os.environ.copy() run_env = os.environ.copy()
@@ -31,8 +56,17 @@ def run_connection_test(name, config):
bufsize=1, bufsize=1,
env=run_env env=run_env
) )
except Exception as e: except Exception as exc:
print(f" [FAIL] Failed to spawn process: {e}") print(f" [FAIL] Failed to spawn process: {exc}")
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
probe_result={"success": False, "error": str(exc)},
process={"profile": env.get("GITEA_MCP_PROFILE"), "env": env},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" diagnostics: {json.dumps(assessment['diagnostics'], sort_keys=True)}")
return False return False
# Send initialize request # Send initialize request
@@ -48,26 +82,36 @@ def run_connection_test(name, config):
} }
try: try:
proc.stdin.write(json.dumps(init_req) + "\n") _write_message(proc, init_req)
proc.stdin.flush()
# Read response # Read response
line = proc.stdout.readline() res, stderr_content = _read_json_line(proc)
if not line: if res is None:
stderr_content = proc.stderr.read()
print(f" [FAIL] Received EOF from process. Stderr:\n{stderr_content}") print(f" [FAIL] Received EOF from process. Stderr:\n{stderr_content}")
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
probe_result={"success": False, "error": stderr_content or "EOF"},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" remediation: {' '.join(assessment['remediation'])}")
proc.terminate() proc.terminate()
return False return False
print(f" [OK] Received initialize response: {line.strip()[:150]}...") print(f" [OK] Received initialize response: {str(res)[:150]}...")
# Send initialized notification # Send initialized notification
init_notif = { init_notif = {
"jsonrpc": "2.0", "jsonrpc": "2.0",
"method": "notifications/initialized" "method": "notifications/initialized"
} }
proc.stdin.write(json.dumps(init_notif) + "\n") _write_message(proc, init_notif)
proc.stdin.flush()
# Send tools/list request # Send tools/list request
list_req = { list_req = {
@@ -76,16 +120,27 @@ def run_connection_test(name, config):
"params": {}, "params": {},
"id": 2 "id": 2
} }
proc.stdin.write(json.dumps(list_req) + "\n") _write_message(proc, list_req)
proc.stdin.flush()
line = proc.stdout.readline() res, stderr_content = _read_json_line(proc)
if not line: if res is None:
print(" [FAIL] Received EOF on tools/list request.") print(" [FAIL] Received EOF on tools/list request.")
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
probe_result={"success": False, "error": stderr_content or "EOF"},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" remediation: {' '.join(assessment['remediation'])}")
proc.terminate() proc.terminate()
return False return False
res = json.loads(line)
if "error" in res: if "error" in res:
print(f" [FAIL] Server returned error: {res['error']}") print(f" [FAIL] Server returned error: {res['error']}")
proc.terminate() proc.terminate()
@@ -94,16 +149,115 @@ def run_connection_test(name, config):
tools = res.get("result", {}).get("tools", []) tools = res.get("result", {}).get("tools", [])
tool_names = [t.get("name") for t in tools] tool_names = [t.get("name") for t in tools]
print(f" [OK] Successfully retrieved {len(tool_names)} tools: {tool_names[:5]}...") print(f" [OK] Successfully retrieved {len(tool_names)} tools: {tool_names[:5]}...")
if tool_name not in tool_names:
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
registered_tools=tool_names,
probe_result={"success": False, "error": "required tool missing"},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" [FAIL] Required tool '{tool_name}' is not registered.")
print(f" remediation: {' '.join(assessment['remediation'])}")
proc.terminate()
return False
call_req = {
"jsonrpc": "2.0",
"method": "tools/call",
"params": {"name": tool_name, "arguments": {}},
"id": 3,
}
_write_message(proc, call_req)
call_res, stderr_content = _read_json_line(proc)
if call_res is None:
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
registered_tools=tool_names,
probe_result={"success": False, "error": stderr_content or "EOF"},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" [FAIL] Received EOF on {tool_name} invocation.")
print(f" diagnostics: {json.dumps(assessment['diagnostics'], sort_keys=True)}")
print(f" remediation: {' '.join(assessment['remediation'])}")
proc.terminate()
return False
if "error" in call_res:
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
registered_tools=tool_names,
probe_result={"success": False, "error": call_res["error"]},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" [FAIL] {tool_name} invocation returned error: {call_res['error']}")
print(f" remediation: {' '.join(assessment['remediation'])}")
proc.terminate()
return False
assessment = classify_namespace_probe(
name,
required_tool=tool_name,
registered_tools=tool_names,
probe_result={"success": True, "result": call_res.get("result")},
process={
"pid": proc.pid,
"profile": env.get("GITEA_MCP_PROFILE"),
"env": env,
},
config_path=config_path,
probe_source="offline_spawn",
)
print(f" [OK] Successfully invoked required tool '{tool_name}'.")
print(f" diagnostics: {json.dumps(assessment['diagnostics'], sort_keys=True)}")
proc.terminate() proc.terminate()
return True return True
except Exception as e: except Exception as exc:
print(f" [FAIL] Error during handshake: {e}") print(f" [FAIL] Error during handshake: {exc}")
proc.terminate() proc.terminate()
return False return False
def main(): def main():
config_path = "/Users/jasonwalker/.gemini/config/mcp_config.json" parser = argparse.ArgumentParser()
parser.add_argument(
"--config",
default=os.environ.get(
"MCP_CONFIG_PATH",
os.path.expanduser("~/.gemini/config/mcp_config.json"),
),
help="Path to MCP config JSON.",
)
parser.add_argument(
"--namespace",
action="append",
dest="namespaces",
help="Namespace to test. May be repeated.",
)
args = parser.parse_args()
config_path = args.config
try: try:
with open(config_path) as f: with open(config_path) as f:
mcp_config = json.load(f) mcp_config = json.load(f)
@@ -112,10 +266,16 @@ def main():
sys.exit(1) sys.exit(1)
servers = mcp_config.get("mcpServers", {}) servers = mcp_config.get("mcpServers", {})
namespaces = args.namespaces or list(REQUIRED_NAMESPACE_TOOLS)
failed = False failed = False
for name in ["gitea-author", "gitea-reviewer"]: for name in namespaces:
if name in servers: if name in servers:
if not run_connection_test(name, servers[name]): if not run_connection_test(
name,
servers[name],
required_tool=REQUIRED_NAMESPACE_TOOLS.get(name),
config_path=config_path,
):
failed = True failed = True
else: else:
print(f"Server '{name}' not found in mcp_config.json") print(f"Server '{name}' not found in mcp_config.json")
+44 -1
View File
@@ -1,4 +1,5 @@
"""Shared pytest fixtures for the Gitea-Tools test suite.""" """Shared pytest fixtures for the Gitea-Tools test suite."""
import os
import sys import sys
import pytest import pytest
@@ -16,13 +17,46 @@ def _reset_mutation_authority(monkeypatch):
the next test. It must never replace verify_mutation_authority with a the next test. It must never replace verify_mutation_authority with a
no-op: individual tests that need a specific authority state set it up no-op: individual tests that need a specific authority state set it up
explicitly. explicitly.
Session-state isolation (#559 / #590 / #594): many tests use
``patch.dict(os.environ, ..., clear=True)``, which drops
``GITEA_MCP_SESSION_STATE_DIR``. Relying only on the env var therefore
re-exposes the operator host cache
(``~/.cache/gitea-tools/session-state``) and its review-mutation ledger.
Pin ``default_state_dir`` / ``DEFAULT_STATE_DIR`` to a per-test temp dir
so durable load/save never touches host state even after env clears.
""" """
monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False) monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False)
# Isolate durable session-state files so tests never share host cache (#559). # Isolate durable session-state files so tests never share host cache (#559).
import tempfile import tempfile
_state_tmp = tempfile.TemporaryDirectory(prefix="gitea-session-state-") _state_tmp = tempfile.TemporaryDirectory(prefix="gitea-session-state-")
monkeypatch.setenv("GITEA_MCP_SESSION_STATE_DIR", _state_tmp.name) state_dir = _state_tmp.name
monkeypatch.setenv("GITEA_MCP_SESSION_STATE_DIR", state_dir)
try:
import mcp_session_state
except Exception:
mcp_session_state = None
if mcp_session_state is not None:
# Survive patch.dict(..., clear=True) that drops the env var (#590).
# Still honor an explicit GITEA_MCP_SESSION_STATE_DIR when tests set
# their own temp dir (see tests/test_mcp_session_state.py).
monkeypatch.setattr(
mcp_session_state, "DEFAULT_STATE_DIR", state_dir, raising=False
)
def _isolated_default_state_dir(
_fallback: str = state_dir,
_env_key: str = mcp_session_state.STATE_DIR_ENV,
) -> str:
raw = (os.environ.get(_env_key) or "").strip()
return raw or _fallback
monkeypatch.setattr(
mcp_session_state,
"default_state_dir",
_isolated_default_state_dir,
)
try: try:
import mcp_server import mcp_server
except Exception: except Exception:
@@ -32,6 +66,7 @@ def _reset_mutation_authority(monkeypatch):
monkeypatch.setattr(mcp_server, "_MUTATION_AUTHORITY", None) monkeypatch.setattr(mcp_server, "_MUTATION_AUTHORITY", None)
monkeypatch.setattr(mcp_server, "_IDENTITY_CACHE", {}) monkeypatch.setattr(mcp_server, "_IDENTITY_CACHE", {})
monkeypatch.setattr(mcp_server, "_REVIEW_DECISION_LOCK", None) monkeypatch.setattr(mcp_server, "_REVIEW_DECISION_LOCK", None)
monkeypatch.setattr(mcp_server, "_LIVE_NAMESPACE_HEALTH", {})
monkeypatch.setattr(mcp_server, "_preflight_whoami_called", False) monkeypatch.setattr(mcp_server, "_preflight_whoami_called", False)
monkeypatch.setattr(mcp_server, "_preflight_capability_called", False) monkeypatch.setattr(mcp_server, "_preflight_capability_called", False)
monkeypatch.setattr(mcp_server, "_preflight_resolved_role", None) monkeypatch.setattr(mcp_server, "_preflight_resolved_role", None)
@@ -40,6 +75,14 @@ def _reset_mutation_authority(monkeypatch):
monkeypatch.setattr(mcp_server, "_preflight_capability_baseline_porcelain", None) monkeypatch.setattr(mcp_server, "_preflight_capability_baseline_porcelain", None)
monkeypatch.setattr(mcp_server, "_preflight_resolved_task", None) monkeypatch.setattr(mcp_server, "_preflight_resolved_task", None)
monkeypatch.setattr(mcp_server, "_preflight_reviewer_violation_files", []) monkeypatch.setattr(mcp_server, "_preflight_reviewer_violation_files", [])
# Unit tests must not depend on live host MCP process health. Tests that
# use patch.dict(..., clear=True) drop PYTEST_CURRENT_TEST, which would
# otherwise re-enable the stale-runtime probe against operator daemons.
monkeypatch.setattr(
mcp_server,
"_check_mcp_runtimes_diagnostics",
lambda *args, **kwargs: [],
)
try: try:
import review_workflow_load import review_workflow_load
review_workflow_load._REVIEW_WORKFLOW_LOAD = None review_workflow_load._REVIEW_WORKFLOW_LOAD = None
+366
View File
@@ -0,0 +1,366 @@
"""Tests for controller-owned allocator (#600) on control-plane DB (#613)."""
from __future__ import annotations
import os
import tempfile
import threading
import unittest
from concurrent.futures import ThreadPoolExecutor, as_completed
from allocator_service import (
OUTCOME_ASSIGNED,
OUTCOME_BLOCKED_TERMINAL,
OUTCOME_NO_SAFE,
OUTCOME_PREVIEW,
OUTCOME_WAIT,
WorkCandidate,
allocate_next_work,
candidate_from_dict,
classify_skip,
expected_role_for_candidate,
)
from control_plane_db import ControlPlaneDB, InvalidWorkKindError
class AllocatorServiceTest(unittest.TestCase):
def setUp(self) -> None:
self._tmp = tempfile.TemporaryDirectory()
self.db_path = os.path.join(self._tmp.name, "cp.sqlite3")
self.db = ControlPlaneDB(self.db_path)
def tearDown(self) -> None:
self._tmp.cleanup()
def _alloc(self, **kwargs):
defaults = dict(
db=self.db,
session_id="s-test",
role="author",
remote="prgs",
org="org",
repo="repo",
candidates=[],
apply=False,
profile_name="prgs-author",
username="jcwalker3",
)
defaults.update(kwargs)
return allocate_next_work(**defaults)
def test_selects_ready_issue_for_author(self) -> None:
cands = [
WorkCandidate(
kind="issue",
number=612,
labels=("status:ready",),
title="bridge",
dependency_unmet=True,
dependency_reason="downstream of #600",
),
WorkCandidate(
kind="issue",
number=600,
labels=("status:ready", "type:feature"),
title="allocator",
priority=50,
),
WorkCandidate(
kind="issue",
number=601,
labels=("status:blocked",),
title="blocked",
blocked=True,
),
]
res = self._alloc(candidates=cands, apply=False)
self.assertTrue(res["success"])
self.assertEqual(res["outcome"], OUTCOME_PREVIEW)
self.assertEqual(res["selected"]["number"], 600)
self.assertEqual(res["selected"]["kind"], "issue")
# When 600 is highest priority valid work, lower-priority blocked/dep
# candidates are not visited. Prove they are skipped when they sort first.
res2 = self._alloc(
candidates=[
WorkCandidate(
kind="issue",
number=612,
labels=("status:ready",),
priority=99,
dependency_unmet=True,
dependency_reason="downstream of #600",
),
WorkCandidate(
kind="issue",
number=601,
labels=("status:blocked",),
priority=98,
blocked=True,
),
WorkCandidate(
kind="issue",
number=600,
labels=("status:ready",),
priority=1,
),
],
apply=False,
)
skipped_nums = {s["number"] for s in res2["skipped"]}
self.assertIn(612, skipped_nums)
self.assertIn(601, skipped_nums)
self.assertEqual(res2["selected"]["number"], 600)
self.assertIsNone(res["assignment"])
self.assertFalse(res["file_lock_only"])
self.assertFalse(res["comment_lease_only"])
def test_atomic_assign_and_lease_on_apply(self) -> None:
cands = [
WorkCandidate(
kind="issue",
number=600,
labels=("status:ready",),
priority=10,
)
]
res = self._alloc(candidates=cands, apply=True, session_id="s-a")
self.assertEqual(res["outcome"], OUTCOME_ASSIGNED)
asn = res["assignment"]
self.assertEqual(asn["outcome"], "assigned")
self.assertIsNotNone(asn["assignment_id"])
self.assertIsNotNone(asn["lease_id"])
self.assertEqual(asn["work_number"], 600)
self.assertIn("implement", asn["allowed_actions"])
self.assertIn("merge", asn["forbidden_actions"])
proof = res["lease_proof"]
self.assertEqual(proof["source"], "control_plane_db.assign_and_lease")
def test_concurrent_allocators_no_double_assign(self) -> None:
cand = WorkCandidate(
kind="pr",
number=100,
head_sha="a" * 40,
priority=10,
)
# Seed work item so both race the same target
self.db.upsert_work_item(
remote="prgs",
org="org",
repo="repo",
kind="pr",
number=100,
current_head_sha="a" * 40,
)
results = []
lock = threading.Lock()
def worker(sid: str):
r = allocate_next_work(
self.db,
session_id=sid,
role="reviewer",
remote="prgs",
org="org",
repo="repo",
candidates=[cand],
apply=True,
profile_name="prgs-reviewer",
)
with lock:
results.append(r)
with ThreadPoolExecutor(max_workers=2) as pool:
futs = [pool.submit(worker, f"s-{i}") for i in range(2)]
for f in as_completed(futs):
f.result()
outcomes = [r["outcome"] for r in results]
self.assertEqual(outcomes.count(OUTCOME_ASSIGNED), 1, outcomes)
self.assertEqual(outcomes.count(OUTCOME_WAIT), 1, outcomes)
def test_blocked_and_dependency_skipped(self) -> None:
cands = [
WorkCandidate(kind="issue", number=1, blocked=True, labels=("status:blocked",)),
WorkCandidate(
kind="issue",
number=612,
labels=("status:ready",),
dependency_unmet=True,
dependency_reason="downstream of #600",
),
]
res = self._alloc(candidates=cands, apply=True, role="author")
self.assertEqual(res["outcome"], OUTCOME_NO_SAFE)
self.assertIsNone(res["selected"])
reasons = " ".join(s["reason"] for s in res["skipped"])
self.assertIn("blocked", reasons.lower())
self.assertIn("600", reasons)
def test_already_leased_returns_wait(self) -> None:
self.db.upsert_session(session_id="owner", role="author")
self.db.assign_and_lease(
session_id="owner",
role="author",
remote="prgs",
org="org",
repo="repo",
kind="issue",
number=50,
)
res = self._alloc(
session_id="other",
candidates=[
WorkCandidate(kind="issue", number=50, labels=("status:ready",))
],
apply=True,
)
self.assertEqual(res["outcome"], OUTCOME_WAIT)
self.assertEqual(res["owner_session_id"], "owner")
def test_role_ineligible_skipped(self) -> None:
# PR with current-head REQUEST_CHANGES expects author, not reviewer.
cand = WorkCandidate(
kind="pr",
number=9,
head_sha="b" * 40,
request_changes_current_head=True,
)
self.assertEqual(expected_role_for_candidate(cand), "author")
res = self._alloc(
role="reviewer",
profile_name="prgs-reviewer",
candidates=[cand],
apply=False,
)
self.assertEqual(res["outcome"], OUTCOME_NO_SAFE)
self.assertTrue(any("expects role" in s["reason"] for s in res["skipped"]))
def test_terminal_lock_blocks_downstream_reviewer_prs(self) -> None:
self.db.set_terminal_lock(
remote="prgs",
org="org",
repo="repo",
terminal_pr=10,
decision="request_changes",
status="active",
)
cands = [
WorkCandidate(kind="pr", number=11, head_sha="c" * 40, priority=5),
WorkCandidate(kind="pr", number=10, head_sha="d" * 40, priority=1),
]
res = self._alloc(
role="reviewer",
profile_name="prgs-reviewer",
candidates=cands,
apply=False,
)
# Terminal PR #10 is selectable; #11 skipped for terminal path.
self.assertEqual(res["selected"]["number"], 10)
self.assertTrue(
any(
s["number"] == 11 and "terminal-review lock" in s["reason"]
for s in res["skipped"]
)
)
def test_terminal_lock_blocks_all_when_only_downstream(self) -> None:
self.db.set_terminal_lock(
remote="prgs",
org="org",
repo="repo",
terminal_pr=10,
decision="request_changes",
status="active",
)
res = self._alloc(
role="reviewer",
profile_name="prgs-reviewer",
candidates=[
WorkCandidate(kind="pr", number=99, head_sha="e" * 40),
],
apply=False,
)
self.assertEqual(res["outcome"], OUTCOME_BLOCKED_TERMINAL)
def test_no_work_structured(self) -> None:
res = self._alloc(candidates=[], apply=True)
self.assertEqual(res["outcome"], OUTCOME_NO_SAFE)
self.assertIsNone(res["selected"])
self.assertTrue(res["reasons"])
def test_rejects_incident_kind(self) -> None:
with self.assertRaises(InvalidWorkKindError):
WorkCandidate(kind="sentry_incident", number=1)
def test_612_downstream_marker_in_result(self) -> None:
res = self._alloc(
candidates=[
WorkCandidate(kind="issue", number=600, labels=("status:ready",))
],
apply=True,
)
self.assertIn("612", res.get("downstream_note", ""))
def test_substrate_not_file_or_comment_lease(self) -> None:
res = self._alloc(
candidates=[
WorkCandidate(kind="issue", number=1, labels=("status:ready",))
],
apply=True,
)
self.assertEqual(res["substrate"], "control_plane_db")
self.assertFalse(res["file_lock_only"])
self.assertFalse(res["comment_lease_only"])
self.assertEqual(
res["lease_proof"]["source"], "control_plane_db.assign_and_lease"
)
def test_candidate_from_dict(self) -> None:
c = candidate_from_dict(
{
"kind": "pr",
"number": 7,
"head_sha": "f" * 40,
"approval_on_current_head": True,
"mergeable": True,
}
)
self.assertEqual(expected_role_for_candidate(c), "merger")
def test_merger_gets_clean_approval(self) -> None:
c = WorkCandidate(
kind="pr",
number=3,
head_sha="1" * 40,
approval_on_current_head=True,
mergeable=True,
priority=100,
)
res = self._alloc(
role="merger",
profile_name="prgs-merger",
candidates=[c],
apply=True,
session_id="merger-1",
)
self.assertEqual(res["outcome"], OUTCOME_ASSIGNED)
self.assertEqual(res["assignment"]["work_number"], 3)
self.assertIn("merge", res["assignment"]["allowed_actions"])
def test_db_unavailable_fails_closed(self) -> None:
res = allocate_next_work(
None, # type: ignore[arg-type]
session_id="x",
role="author",
remote="prgs",
org="o",
repo="r",
candidates=[],
apply=True,
)
self.assertFalse(res["success"])
self.assertIn("unavailable", res["reasons"][0].lower())
if __name__ == "__main__":
unittest.main()
+824
View File
@@ -0,0 +1,824 @@
"""Tests for control-plane DB substrate (#613)."""
from __future__ import annotations
import os
import tempfile
import threading
import unittest
from concurrent.futures import ThreadPoolExecutor, as_completed
from datetime import timedelta
from control_plane_db import (
ControlPlaneDB,
InvalidWorkKindError,
LeaseRequiredError,
WORK_KINDS,
_ts,
_utc_now,
)
class ControlPlaneDBTest(unittest.TestCase):
def setUp(self) -> None:
self._tmp = tempfile.TemporaryDirectory()
self.db_path = os.path.join(self._tmp.name, "cp.sqlite3")
self.db = ControlPlaneDB(self.db_path)
def tearDown(self) -> None:
self._tmp.cleanup()
def test_schema_and_architecture_meta(self) -> None:
import sqlite3
conn = sqlite3.connect(self.db_path)
try:
rows = dict(conn.execute("SELECT key, value FROM schema_meta").fetchall())
finally:
conn.close()
self.assertEqual(rows["schema_version"], "2")
self.assertIn("DB coordinates", rows["architecture"])
self.assertIn("bridge", rows["architecture"].lower())
def test_rejects_raw_incident_as_work_kind(self) -> None:
with self.assertRaises(InvalidWorkKindError):
self.db.upsert_work_item(
remote="prgs",
org="org",
repo="repo",
kind="sentry_incident",
number=1,
)
with self.assertRaises(InvalidWorkKindError):
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="org",
repo="repo",
kind="glitchtip_incident",
number=9,
)
self.assertEqual(WORK_KINDS, frozenset({"issue", "pr"}))
def test_atomic_assign_and_lease_fields(self) -> None:
self.db.upsert_session(session_id="s-a", role="author", profile="prgs-author")
result = self.db.assign_and_lease(
session_id="s-a",
role="author",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
kind="issue",
number=613,
expected_head_sha="abc123",
allowed_actions=("implement", "comment"),
forbidden_actions=("approve", "merge"),
)
self.assertEqual(result.outcome, "assigned")
self.assertIsNotNone(result.assignment_id)
self.assertIsNotNone(result.lease_id)
self.assertEqual(result.role, "author")
self.assertEqual(result.work_kind, "issue")
self.assertEqual(result.work_number, 613)
self.assertEqual(result.expected_head_sha, "abc123")
self.assertIn("implement", result.allowed_actions)
self.assertIn("merge", result.forbidden_actions)
self.assertIsNotNone(result.expires_at)
def test_second_session_waits_on_foreign_lease(self) -> None:
self.db.upsert_session(session_id="s1", role="author")
self.db.upsert_session(session_id="s2", role="author")
first = self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=100,
expected_head_sha="deadbeef",
)
self.assertEqual(first.outcome, "assigned")
second = self.db.assign_and_lease(
session_id="s2",
role="author",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=100,
expected_head_sha="deadbeef",
)
self.assertEqual(second.outcome, "wait")
self.assertEqual(second.owner_session_id, "s1")
def test_owner_resume_refreshes_lease(self) -> None:
self.db.upsert_session(session_id="s1", role="reviewer")
a = self.db.assign_and_lease(
session_id="s1",
role="reviewer",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=50,
expected_head_sha="head-50",
)
b = self.db.assign_and_lease(
session_id="s1",
role="reviewer",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=50,
expected_head_sha="head-50",
)
self.assertEqual(b.outcome, "assigned")
self.assertEqual(b.lease_id, a.lease_id)
self.assertIn("owner-resume", b.reason)
def test_require_valid_assignment_gates_mutations(self) -> None:
self.db.upsert_session(session_id="s1", role="author")
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=7,
allowed_actions=("implement",),
forbidden_actions=("merge",),
)
proof = self.db.require_valid_assignment(
session_id="s1",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=7,
action="implement",
)
self.assertEqual(proof["session_id"], "s1")
with self.assertRaises(LeaseRequiredError):
self.db.require_valid_assignment(
session_id="s1",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=7,
action="merge",
)
with self.assertRaises(LeaseRequiredError):
self.db.require_valid_assignment(
session_id="s-other",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=7,
action="implement",
)
def test_expired_lease_allows_reassign(self) -> None:
self.db.upsert_session(session_id="s1", role="author")
self.db.upsert_session(session_id="s2", role="author")
past = _utc_now() - timedelta(hours=1)
# Create lease already expired by using negative TTL edge via direct assign then expire
assigned = self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=3,
lease_ttl_seconds=1,
)
self.assertEqual(assigned.outcome, "assigned")
# Force expiry in DB
import sqlite3
conn = sqlite3.connect(self.db_path)
try:
conn.execute(
"UPDATE leases SET expires_at = ? WHERE lease_id = ?",
(_ts(past), assigned.lease_id),
)
conn.commit()
finally:
conn.close()
n = self.db.expire_stale_leases()
self.assertGreaterEqual(n, 1)
second = self.db.assign_and_lease(
session_id="s2",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=3,
)
self.assertEqual(second.outcome, "assigned")
self.assertEqual(second.session_id, "s2")
def test_merged_work_never_assigned(self) -> None:
self.db.upsert_session(session_id="s1", role="merger")
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="pr",
number=99,
state="merged",
)
result = self.db.assign_and_lease(
session_id="s1",
role="merger",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=99,
expected_head_sha="merged-head",
)
self.assertEqual(result.outcome, "no_safe_work")
def test_four_concurrent_sessions_unique_assignments(self) -> None:
"""Four concurrent assigners on four different issues — all succeed uniquely.
Also two concurrent assigners on the *same* issue: at most one assigned.
"""
for i in range(4):
self.db.upsert_session(session_id=f"sess-{i}", role="author")
def claim_unique(i: int):
return self.db.assign_and_lease(
session_id=f"sess-{i}",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=1000 + i,
)
with ThreadPoolExecutor(max_workers=4) as pool:
results = [f.result() for f in as_completed([pool.submit(claim_unique, i) for i in range(4)])]
self.assertEqual({r.outcome for r in results}, {"assigned"})
numbers = sorted(r.work_number for r in results)
self.assertEqual(numbers, [1000, 1001, 1002, 1003])
# Contention on one item
self.db.upsert_session(session_id="c1", role="author")
self.db.upsert_session(session_id="c2", role="author")
self.db.upsert_session(session_id="c3", role="author")
self.db.upsert_session(session_id="c4", role="author")
barrier = threading.Barrier(4)
outcomes: list[str] = []
lock = threading.Lock()
def contend(sid: str) -> None:
barrier.wait()
res = self.db.assign_and_lease(
session_id=sid,
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=7777,
)
with lock:
outcomes.append(res.outcome)
threads = [threading.Thread(target=contend, args=(f"c{i}",)) for i in range(1, 5)]
for t in threads:
t.start()
for t in threads:
t.join()
self.assertEqual(outcomes.count("assigned"), 1)
self.assertEqual(outcomes.count("wait"), 3)
def test_terminal_lock_index(self) -> None:
self.db.set_terminal_lock(
remote="prgs",
org="o",
repo="r",
terminal_pr=332,
review_id="rev-1",
decision="approve",
)
row = self.db.get_active_terminal_lock(remote="prgs", org="o", repo="r")
self.assertIsNotNone(row)
assert row is not None
self.assertEqual(row["terminal_pr"], 332)
self.assertEqual(row["status"], "active")
def test_incident_links_not_work_items(self) -> None:
link = self.db.upsert_incident_link(
provider="sentry",
provider_base_url="https://sentry.prgs.cc",
provider_org="prgs",
provider_project="gitea-tools-mcp",
provider_issue_id="12345",
gitea_org="Scaled-Tech-Consulting",
gitea_repo="Gitea-Tools",
gitea_issue_number=9001,
fingerprint="fp-1",
)
self.assertEqual(link["gitea_issue_number"], 9001)
found = self.db.get_incident_link_for_gitea_issue(
gitea_org="Scaled-Tech-Consulting",
gitea_repo="Gitea-Tools",
gitea_issue_number=9001,
)
self.assertIsNotNone(found)
# Linking does not create a work_item of incident kind
with self.assertRaises(InvalidWorkKindError):
self.db.upsert_work_item(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
kind="sentry",
number=12345,
)
def test_heartbeat_and_release(self) -> None:
self.db.upsert_session(session_id="s1", role="author")
a = self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=1,
)
hb = self.db.heartbeat_lease(a.lease_id, session_id="s1")
self.assertEqual(hb["lease_id"], a.lease_id)
self.db.release_lease(a.lease_id, session_id="s1")
# After release another session can claim
self.db.upsert_session(session_id="s2", role="author")
b = self.db.assign_and_lease(
session_id="s2",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=1,
)
self.assertEqual(b.outcome, "assigned")
self.assertEqual(b.session_id, "s2")
def test_require_valid_assignment_rejects_stale_head(self) -> None:
"""Assignment must not authorize mutations after work-item head drifts."""
self.db.upsert_session(session_id="s1", role="author")
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=42,
expected_head_sha="head-v1",
allowed_actions=("implement",),
)
# Head drifts after assignment
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="pr",
number=42,
current_head_sha="head-v2",
)
with self.assertRaises(LeaseRequiredError) as ctx:
self.db.require_valid_assignment(
session_id="s1",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=42,
action="implement",
)
self.assertIn("stale head", str(ctx.exception).lower())
def test_require_valid_assignment_rejects_terminal_state(self) -> None:
"""Assignment must not authorize mutations after work item is merged/closed."""
self.db.upsert_session(session_id="s1", role="author")
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=55,
expected_head_sha="abc",
allowed_actions=("implement",),
)
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="pr",
number=55,
state="merged",
current_head_sha="abc",
)
with self.assertRaises(LeaseRequiredError) as ctx:
self.db.require_valid_assignment(
session_id="s1",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=55,
action="implement",
)
self.assertIn("terminal", str(ctx.exception).lower())
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="issue",
number=56,
state="open",
)
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=56,
allowed_actions=("implement",),
)
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="issue",
number=56,
state="closed",
)
with self.assertRaises(LeaseRequiredError):
self.db.require_valid_assignment(
session_id="s1",
remote="prgs",
org="o",
repo="r",
kind="issue",
number=56,
action="implement",
)
def test_pr_assignment_requires_expected_head_sha(self) -> None:
"""PR assign and mutation must fail closed without a head pin."""
self.db.upsert_session(session_id="s1", role="author")
with self.assertRaises(LeaseRequiredError) as ctx:
self.db.assign_and_lease(
session_id="s1",
role="author",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=88,
expected_head_sha=None,
allowed_actions=("implement",),
)
self.assertIn("expected_head_sha", str(ctx.exception))
# Legacy path: force an unpinned PR assignment into the DB, then
# populate head and prove mutation is still rejected.
import sqlite3
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="pr",
number=89,
current_head_sha=None,
)
conn = sqlite3.connect(self.db_path)
try:
wid = conn.execute(
"SELECT work_item_id FROM work_items WHERE kind='pr' AND number=89"
).fetchone()[0]
conn.execute(
"""
INSERT INTO sessions(session_id, role, started_at, last_heartbeat_at, status)
VALUES ('legacy', 'author', '2020-01-01T00:00:00Z', '2020-01-01T00:00:00Z', 'active')
"""
)
conn.execute(
"""
INSERT INTO leases(
lease_id, work_item_id, session_id, role, phase,
expires_at, heartbeat_at, status
) VALUES (
'lease-legacy', ?, 'legacy', 'author', 'claimed',
'2099-01-01T00:00:00Z', '2020-01-01T00:00:00Z', 'active'
)
""",
(wid,),
)
conn.execute(
"""
INSERT INTO assignments(
assignment_id, work_item_id, session_id, lease_id,
allowed_actions, forbidden_actions, expected_head_sha,
role, status, created_at
) VALUES (
'asn-legacy', ?, 'legacy', 'lease-legacy',
'["implement"]', '["merge"]', NULL,
'author', 'active', '2020-01-01T00:00:00Z'
)
""",
(wid,),
)
conn.commit()
finally:
conn.close()
self.db.upsert_work_item(
remote="prgs",
org="o",
repo="r",
kind="pr",
number=89,
current_head_sha="populated-head",
)
with self.assertRaises(LeaseRequiredError) as ctx2:
self.db.require_valid_assignment(
session_id="legacy",
remote="prgs",
org="o",
repo="r",
kind="pr",
number=89,
action="implement",
)
self.assertIn("expected_head_sha pin", str(ctx2.exception))
def test_migrate_duplicate_null_scope_incident_links(self) -> None:
"""Legacy NULL-scope duplicates must migrate without UNIQUE crash."""
import sqlite3
from control_plane_db import ControlPlaneDB, ControlPlaneError
# Build a v1-like table with nullable scope columns and insert dups
# that collapse under normalization, then open ControlPlaneDB on it.
path = os.path.join(self._tmp.name, "legacy_dups.sqlite3")
conn = sqlite3.connect(path)
try:
conn.executescript(
"""
CREATE TABLE schema_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
CREATE TABLE incident_links (
link_id INTEGER PRIMARY KEY AUTOINCREMENT,
provider TEXT NOT NULL,
provider_base_url TEXT,
provider_org TEXT,
provider_project TEXT,
provider_issue_id TEXT NOT NULL,
provider_short_id TEXT,
provider_permalink TEXT,
fingerprint TEXT,
gitea_org TEXT NOT NULL,
gitea_repo TEXT NOT NULL,
gitea_issue_number INTEGER NOT NULL,
linked_pr_numbers TEXT,
first_seen TEXT,
last_seen TEXT,
event_count INTEGER,
status TEXT NOT NULL DEFAULT 'open',
release_resolved_at TEXT,
last_sync_at TEXT,
UNIQUE (
provider, provider_base_url, provider_org,
provider_project, provider_issue_id
)
);
INSERT INTO incident_links(
provider, provider_base_url, provider_org, provider_project,
provider_issue_id, gitea_org, gitea_repo, gitea_issue_number, status
) VALUES
('sentry', NULL, NULL, NULL, 'dup-1', 'org', 'repo', 10, 'open'),
('sentry', NULL, NULL, NULL, 'dup-1', 'org', 'repo', 10, 'open');
"""
)
# SQLite allows two NULL-scope rows with same provider/issue under UNIQUE.
n = conn.execute("SELECT COUNT(*) FROM incident_links").fetchone()[0]
self.assertEqual(n, 2)
conn.commit()
finally:
conn.close()
db = ControlPlaneDB(path)
conn2 = sqlite3.connect(path)
try:
n2 = conn2.execute("SELECT COUNT(*) FROM incident_links").fetchone()[0]
rows = conn2.execute(
"SELECT provider_base_url, provider_org, provider_project, gitea_issue_number "
"FROM incident_links"
).fetchall()
finally:
conn2.close()
self.assertEqual(n2, 1)
self.assertEqual(rows[0][0], "")
self.assertEqual(rows[0][1], "")
self.assertEqual(rows[0][2], "")
self.assertEqual(rows[0][3], 10)
# Touch to silence unused import in type checkers if needed
self.assertTrue(issubclass(ControlPlaneError, Exception))
del db
def test_migrate_conflicting_duplicate_incident_links_fails_closed(self) -> None:
"""Conflicting Gitea targets for the same provider key must fail closed."""
import sqlite3
from control_plane_db import ControlPlaneDB, ControlPlaneError
path = os.path.join(self._tmp.name, "legacy_conflict.sqlite3")
conn = sqlite3.connect(path)
try:
conn.executescript(
"""
CREATE TABLE incident_links (
link_id INTEGER PRIMARY KEY AUTOINCREMENT,
provider TEXT NOT NULL,
provider_base_url TEXT,
provider_org TEXT,
provider_project TEXT,
provider_issue_id TEXT NOT NULL,
gitea_org TEXT NOT NULL,
gitea_repo TEXT NOT NULL,
gitea_issue_number INTEGER NOT NULL,
status TEXT NOT NULL DEFAULT 'open',
UNIQUE (
provider, provider_base_url, provider_org,
provider_project, provider_issue_id
)
);
INSERT INTO incident_links(
provider, provider_base_url, provider_org, provider_project,
provider_issue_id, gitea_org, gitea_repo, gitea_issue_number
) VALUES
('sentry', NULL, NULL, NULL, 'dup-c', 'org', 'repo', 1),
('sentry', NULL, NULL, NULL, 'dup-c', 'org', 'repo', 2);
"""
)
conn.commit()
finally:
conn.close()
with self.assertRaises(ControlPlaneError) as ctx:
ControlPlaneDB(path)
self.assertIn("conflicting", str(ctx.exception).lower())
def test_migrate_conflicting_observation_metadata_fails_closed(self) -> None:
"""Same provider key + same Gitea target but differing obs metadata must fail closed.
Regression for silent data loss: migration used to keep lowest link_id and
delete peers after comparing only Gitea targets (#619 RC3).
"""
import sqlite3
from control_plane_db import ControlPlaneDB, ControlPlaneError
path = os.path.join(self._tmp.name, "legacy_meta_conflict.sqlite3")
conn = sqlite3.connect(path)
try:
conn.executescript(
"""
CREATE TABLE schema_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
CREATE TABLE incident_links (
link_id INTEGER PRIMARY KEY AUTOINCREMENT,
provider TEXT NOT NULL,
provider_base_url TEXT,
provider_org TEXT,
provider_project TEXT,
provider_issue_id TEXT NOT NULL,
provider_short_id TEXT,
provider_permalink TEXT,
fingerprint TEXT,
gitea_org TEXT NOT NULL,
gitea_repo TEXT NOT NULL,
gitea_issue_number INTEGER NOT NULL,
linked_pr_numbers TEXT,
first_seen TEXT,
last_seen TEXT,
event_count INTEGER,
status TEXT NOT NULL DEFAULT 'open',
release_resolved_at TEXT,
last_sync_at TEXT,
UNIQUE (
provider, provider_base_url, provider_org,
provider_project, provider_issue_id
)
);
INSERT INTO incident_links(
provider, provider_base_url, provider_org, provider_project,
provider_issue_id, provider_permalink, fingerprint,
gitea_org, gitea_repo, gitea_issue_number,
event_count, status, first_seen, last_seen
) VALUES
(
'sentry', NULL, NULL, NULL, 'dup-meta',
'https://sentry.example/issues/1', 'fingerprint-A',
'org', 'repo', 42,
1, 'open', '2026-01-01T00:00:00Z', '2026-01-01T01:00:00Z'
),
(
'sentry', NULL, NULL, NULL, 'dup-meta',
'https://sentry.example/issues/1', 'fingerprint-B',
'org', 'repo', 42,
99, 'resolved', '2026-01-01T00:00:00Z', '2026-01-02T00:00:00Z'
);
"""
)
n = conn.execute("SELECT COUNT(*) FROM incident_links").fetchone()[0]
self.assertEqual(n, 2)
conn.commit()
finally:
conn.close()
with self.assertRaises(ControlPlaneError) as ctx:
ControlPlaneDB(path)
msg = str(ctx.exception).lower()
self.assertIn("conflicting", msg)
self.assertIn("observation metadata", msg)
# Rows must still be present — migration must not delete before failing.
conn2 = sqlite3.connect(path)
try:
remaining = conn2.execute("SELECT COUNT(*) FROM incident_links").fetchone()[0]
fps = {
r[0]
for r in conn2.execute(
"SELECT fingerprint FROM incident_links ORDER BY link_id"
).fetchall()
}
finally:
conn2.close()
self.assertEqual(remaining, 2)
self.assertEqual(fps, {"fingerprint-A", "fingerprint-B"})
def test_incident_links_minimal_upsert_is_canonical(self) -> None:
"""Repeated minimal upserts must update one row (NULL-safe uniqueness)."""
a = self.db.upsert_incident_link(
provider="sentry",
provider_issue_id="inc-1",
gitea_org="org",
gitea_repo="repo",
gitea_issue_number=1,
)
b = self.db.upsert_incident_link(
provider="sentry",
provider_issue_id="inc-1",
gitea_org="org",
gitea_repo="repo",
gitea_issue_number=2,
# omit optional scope fields again
)
c = self.db.upsert_incident_link(
provider="sentry",
provider_issue_id="inc-1",
gitea_org="org",
gitea_repo="repo",
gitea_issue_number=3,
provider_base_url=None,
provider_org="",
provider_project=" ",
)
self.assertEqual(a["link_id"], b["link_id"])
self.assertEqual(b["link_id"], c["link_id"])
self.assertEqual(c["gitea_issue_number"], 3)
self.assertEqual(c["provider_base_url"], "")
self.assertEqual(c["provider_org"], "")
self.assertEqual(c["provider_project"], "")
import sqlite3
conn = sqlite3.connect(self.db_path)
try:
n = conn.execute(
"SELECT COUNT(*) FROM incident_links WHERE provider_issue_id = ?",
("inc-1",),
).fetchone()[0]
finally:
conn.close()
self.assertEqual(n, 1)
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,113 @@
"""Controller-closure baseline-proof gate tests (#529, criterion 6)."""
import unittest
from controller_closure_baseline_proof import (
assess_controller_closure_baseline_proof,
)
from premerge_baseline_proof import (
CLEAN_PASS,
PREMERGE_BASELINE_PROVEN_FAILURE,
UNRESOLVED_REGRESSION_RISK,
)
from final_report_validator import assess_final_report_validator
# Complete pre-merge proof fields for a baseline claim (see #533).
_PROOF_FIELDS = (
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
"Exit status: 1\n"
"Failure signature: AssertionError: None is not true\n"
)
class TestControllerClosureBaselineProof(unittest.TestCase):
def test_empty_report_skipped_and_allowed(self):
result = assess_controller_closure_baseline_proof("")
self.assertFalse(result["block"])
self.assertTrue(result["skipped"])
self.assertEqual(result["label"], CLEAN_PASS)
def test_none_report_allowed(self):
result = assess_controller_closure_baseline_proof(None)
self.assertFalse(result["block"])
self.assertTrue(result["skipped"])
def test_clean_pass_closure_allowed(self):
result = assess_controller_closure_baseline_proof(
"Closing #529. Validation: full suite passed, exit status 0. Clean pass."
)
self.assertFalse(result["block"])
self.assertEqual(result["label"], CLEAN_PASS)
def test_expected_preexisting_failure_without_proof_blocked(self):
report = (
"Closing #529. Full suite: 1 failed. This is an expected "
"pre-existing failure, safe to close."
)
result = assess_controller_closure_baseline_proof(report)
self.assertTrue(result["block"])
self.assertFalse(result["proven"])
self.assertEqual(result["label"], UNRESOLVED_REGRESSION_RISK)
self.assertTrue(result["reasons"])
self.assertTrue(result["safe_next_action"])
def test_current_master_reproduction_only_blocked(self):
report = (
"Closing #529. Full suite: 1 failed. Pre-existing baseline failure — "
"reproduced on current master after merge.\n"
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
"Exit status: 1\n"
"Failure signature: AssertionError: None is not true\n"
)
result = assess_controller_closure_baseline_proof(report)
self.assertTrue(result["block"])
def test_proven_baseline_closure_allowed(self):
report = (
"Closing #529. Full suite: 1 failed, an expected pre-existing "
"baseline failure proven on the PR pre-merge base commit.\n"
+ _PROOF_FIELDS
)
result = assess_controller_closure_baseline_proof(report)
self.assertFalse(result["block"])
self.assertEqual(result["label"], PREMERGE_BASELINE_PROVEN_FAILURE)
class TestControllerCloseTaskKind(unittest.TestCase):
"""The composable validator must cover the controller_close task kind."""
def test_controller_close_blocks_unproven_baseline(self):
report = (
"Full suite: 1 failed. Expected pre-existing failure, closing the "
"tracking issue."
)
result = assess_final_report_validator(report, "controller_close")
self.assertTrue(result["blocked"])
self.assertTrue(
any(
"premerge_baseline_proof" in f["rule_id"]
for f in result["findings"]
)
)
def test_controller_close_alias_close_issue(self):
report = (
"Full suite: 1 failed. Expected pre-existing failure, closing the "
"tracking issue."
)
result = assess_final_report_validator(report, "close_issue")
self.assertTrue(result["blocked"])
def test_controller_close_allows_proven_baseline(self):
report = (
"Full suite: 1 failed, expected pre-existing baseline failure proven "
"on the PR pre-merge base commit.\n" + _PROOF_FIELDS
)
result = assess_final_report_validator(report, "controller_close")
self.assertFalse(result["blocked"])
if __name__ == "__main__":
unittest.main()
+97
View File
@@ -781,6 +781,103 @@ class TestMergePrRules(unittest.TestCase):
self.assertEqual(result["grade"], "A") self.assertEqual(result["grade"], "A")
self.assertFalse(result["blocked"]) self.assertFalse(result["blocked"])
def test_manual_seeded_lease_claim_blocks_without_mcp_evidence(self):
lines = [
"## Controller Handoff",
"",
"- Task: merge PR #203",
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
"- Role: merger",
"- Identity: sysadmin / prgs-merger",
"- Issue/PR: #182 / PR #203",
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Files changed: review_proofs.py",
"- Validation: pytest in branches/review-203",
"- Mutations: merge completed",
"- File edits by reviewer: none",
"- Worktree/index mutations: none",
"- Git ref mutations: git fetch prgs master",
"- MCP/Gitea mutations: merge completed",
"- Review mutations: none",
"- Merge mutations: merged using manually seeded lease proof",
"- Cleanup mutations: none",
"- External-state mutations: none",
"- Read-only diagnostics: metadata check",
"- Current status: PR merged",
"- Blockers: none",
"- Next: none",
"- Safety: review and merge are separate roles",
"- Selected PR: #203",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
"- Active profile: prgs-merger",
"- Role kind: merger",
"- Merge capability source: profile",
"- Explicit operator authorization: MERGE PR 203",
"- Expected head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Approval at current head: yes",
"- Merge result: PR merged successfully",
"- Cleanup status: complete",
]
report = "\n".join(lines)
result = assess_final_report_validator(report, "merge_pr")
self.assertTrue(result["blocked"])
codes = {f.get("rule_id") for f in result.get("findings") or []}
self.assertIn("shared.manual_lease_proof_handoff", codes)
def test_manual_seeded_claim_allowed_when_adoption_cited(self):
lines = [
"## Controller Handoff",
"",
"- Task: merge PR #203",
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
"- Role: merger",
"- Identity: sysadmin / prgs-merger",
"- Issue/PR: #182 / PR #203",
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Files changed: review_proofs.py",
"- Validation: pytest in branches/review-203",
"- Mutations: merge completed",
"- File edits by reviewer: none",
"- Worktree/index mutations: none",
"- Git ref mutations: git fetch prgs master",
"- MCP/Gitea mutations: gitea_adopt_merger_pr_lease then merge",
"- Review mutations: none",
"- Merge mutations: historical manual seed replaced by sanctioned adoption",
"- Cleanup mutations: none",
"- External-state mutations: none",
"- Read-only diagnostics: metadata check",
"- Current status: PR merged",
"- Blockers: none",
"- Next: none",
"- Safety: review and merge are separate roles",
"- Selected PR: #203",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before approval: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Final live head SHA before merge: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Push occurred during validation: no",
"- Active profile: prgs-merger",
"- Role kind: merger",
"- Merge capability source: profile",
"- Explicit operator authorization: MERGE PR 203",
"- Expected head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Approval at current head: yes",
"- lease_proof_source: gitea_adopt_merger_pr_lease",
"- lease_proof_kind: sanctioned_adoption",
"- adoption_comment_id: 8288",
"- Merge result: PR merged successfully",
"- Cleanup status: complete",
]
report = "\n".join(lines)
result = assess_final_report_validator(report, "merge_pr")
self.assertFalse(result["blocked"])
codes = {f.get("rule_id") for f in result.get("findings") or []}
self.assertNotIn("shared.manual_lease_proof_handoff", codes)
def test_missing_merger_fields_blocks(self): def test_missing_merger_fields_blocks(self):
lines = [ lines = [
"## Controller Handoff", "## Controller Handoff",
@@ -0,0 +1,397 @@
"""Head-scoped #332 review-decision locks (#620).
Proves:
* REQUEST_CHANGES on head A does not block mark_ready/submit on head B
* APPROVE on head B is allowed after RC on head A (same open PR)
* same-head duplicate terminal remains blocked
* open-PR cleanup remains forbidden; assessment reports stale_by_head
* historical mutations keep their head_sha (preserved ledger)
* PR #619-style: old RC at 036b78e…, current head 2429d9d…, approve allowed
"""
from __future__ import annotations
import os
import unittest
from unittest.mock import patch
import mcp_server
import stale_review_decision_lock as srdl
HEAD_A = "036b78e31ea5d036452b3a52e0e086b01e0c763f"
HEAD_B = "2429d9d2e826e519eb8eb4ade45987c00838aefb"
HEAD_C = "c" * 40
def _lock(mutations=None, correction=False, ready_pr=None, ready_head=None, ready_action=None):
return {
"task": "review_pr",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"session_pid": os.getpid(),
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"profile_identity": "prgs-reviewer",
"final_review_decision_ready": False,
"ready_pr_number": ready_pr,
"ready_action": ready_action,
"ready_expected_head_sha": ready_head,
"ready_remote": "prgs" if ready_pr else None,
"ready_org": "Scaled-Tech-Consulting" if ready_pr else None,
"ready_repo": "Gitea-Tools" if ready_pr else None,
"live_mutations": list(mutations or []),
"correction_authorized": correction,
"correction_reason": None,
}
RC_619_LEGACY = {
"pr_number": 619,
"action": "request_changes",
"review_id": 410,
"review_state": "request_changes",
# no head_sha — pre-#620 durable ledger shape
}
RC_619_HEAD_A = {
"pr_number": 619,
"action": "request_changes",
"review_id": 410,
"review_state": "request_changes",
"head_sha": HEAD_A,
}
APPROVE_619_HEAD_B = {
"pr_number": 619,
"action": "approve",
"review_id": 411,
"review_state": "approve",
"head_sha": HEAD_B,
}
def _seed(mutations=None, **kwargs):
import review_workflow_load
review_workflow_load.record_review_workflow_load(mcp_server.PROJECT_ROOT)
mcp_server._save_review_decision_lock(_lock(mutations, **kwargs))
mcp_server.gitea_load_review_workflow()
def _open_pr(pr_number=619, head=HEAD_B):
return {
"number": pr_number,
"state": "open",
"merged": False,
"merged_at": None,
"head": {"sha": head},
}
def _no_lease():
return {"block": False, "reasons": [], "mutation_allowed": True}
def _feedback(blocking=False, stale=False, success=True):
return {
"success": success,
"has_blocking_change_requests": blocking,
"review_feedback_stale": stale,
"current_head_sha": HEAD_B,
}
class TestPureHeadScopeHelpers(unittest.TestCase):
def test_mutation_head_prefers_recorded_sha(self):
m = {"pr_number": 1, "head_sha": HEAD_A}
self.assertEqual(srdl.mutation_head_sha(m), HEAD_A)
def test_legacy_mutation_uses_ready_expected_for_same_pr(self):
lock = _lock(
[RC_619_LEGACY],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
self.assertEqual(srdl.mutation_head_sha(RC_619_LEGACY, lock), HEAD_A)
def test_legacy_mutation_ignores_ready_for_other_pr(self):
lock = _lock([RC_619_LEGACY], ready_pr=1, ready_head=HEAD_A)
self.assertIsNone(srdl.mutation_head_sha(RC_619_LEGACY, lock))
def test_prior_blocks_same_head_not_other_head(self):
lock = _lock([RC_619_HEAD_A])
self.assertTrue(
srdl.prior_live_mutations_block_boundary(
lock, pr_number=619, expected_head_sha=HEAD_A
)
)
self.assertFalse(
srdl.prior_live_mutations_block_boundary(
lock, pr_number=619, expected_head_sha=HEAD_B
)
)
def test_backfill_stamps_legacy_terminals(self):
lock = _lock(
[dict(RC_619_LEGACY)],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
srdl.backfill_terminal_heads_from_ready(lock)
self.assertEqual(lock["live_mutations"][0]["head_sha"], HEAD_A)
class TestHardStopHeadScope(unittest.TestCase):
def tearDown(self):
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def test_rc_head_a_allows_mark_ready_head_b(self):
_seed(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(
619, "mark_ready", expected_head_sha=HEAD_B
),
[],
)
def test_rc_head_a_blocks_mark_ready_same_head(self):
_seed(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
reasons = mcp_server.terminal_review_hard_stop_reasons(
619, "mark_ready", expected_head_sha=HEAD_A
)
self.assertTrue(reasons)
self.assertIn("#332", reasons[0])
def test_rc_head_a_blocks_other_pr(self):
_seed([RC_619_HEAD_A], ready_pr=619, ready_head=HEAD_A)
reasons = mcp_server.terminal_review_hard_stop_reasons(
700, "mark_ready", expected_head_sha=HEAD_B
)
self.assertTrue(reasons)
def test_legacy_619_ledger_allows_new_head(self):
"""PR #619-style durable lock without mutation head_sha."""
_seed(
[RC_619_LEGACY],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(
619, "mark_ready", expected_head_sha=HEAD_B
),
[],
)
class TestMarkFinalAndRecord(unittest.TestCase):
def tearDown(self):
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def _mark(self, pr, action, head, feedback=None):
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_get_pr_review_feedback",
return_value=feedback or _feedback(blocking=False, stale=True),
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"eligible": True, "head_sha": head},
):
return mcp_server.gitea_mark_final_review_decision(
pr_number=pr,
action=action,
expected_head_sha=head,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
def test_rc_then_rc_on_new_head(self):
_seed(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
# Prior RC at head A is unresolved but head moved → feedback stale.
res = self._mark(
619,
"request_changes",
HEAD_B,
feedback=_feedback(blocking=True, stale=True),
)
self.assertTrue(res.get("marked_ready"), res)
lock = mcp_server._load_review_decision_lock()
# Historical head A preserved on mutation after backfill.
heads = {
m.get("head_sha")
for m in lock["live_mutations"]
if m.get("action") == "request_changes"
}
self.assertIn(HEAD_A, heads)
self.assertEqual(lock["ready_expected_head_sha"], HEAD_B)
def test_rc_then_approve_on_new_head(self):
_seed(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(619, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
self.assertTrue(res.get("head_scoped"))
def test_same_head_rc_still_blocked_by_hard_stop(self):
_seed(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(619, "approve", HEAD_A)
self.assertFalse(res.get("marked_ready"))
self.assertTrue(any("#332" in r for r in res.get("reasons") or []))
def test_pr619_style_legacy_lock_approve_on_current_head(self):
_seed(
[RC_619_LEGACY],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(619, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
lock = mcp_server._load_review_decision_lock()
# Backfill should have stamped HEAD_A onto the legacy mutation.
self.assertEqual(lock["live_mutations"][0].get("head_sha"), HEAD_A)
self.assertEqual(lock["ready_expected_head_sha"], HEAD_B)
def test_record_live_mutation_stores_head(self):
_seed(ready_pr=619, ready_head=HEAD_B, ready_action="approve")
lock = mcp_server._load_review_decision_lock()
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = 619
lock["ready_expected_head_sha"] = HEAD_B
mcp_server._save_review_decision_lock(lock)
mcp_server.record_live_review_mutation(619, "approve", review_id=99)
stored = mcp_server._load_review_decision_lock()["live_mutations"][-1]
self.assertEqual(stored["head_sha"], HEAD_B)
self.assertEqual(stored["pr_number"], 619)
def test_submit_gate_allows_new_head_after_prior_terminal(self):
"""check_review_decision_gate must not block different-head submit."""
mutations = [RC_619_HEAD_A]
_seed(
mutations,
ready_pr=619,
ready_head=HEAD_B,
ready_action="approve",
)
lock = mcp_server._load_review_decision_lock()
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = 619
lock["ready_action"] = "approve"
lock["ready_expected_head_sha"] = HEAD_B
lock["ready_remote"] = "prgs"
lock["ready_org"] = "Scaled-Tech-Consulting"
lock["ready_repo"] = "Gitea-Tools"
mcp_server._save_review_decision_lock(lock)
reasons = mcp_server.check_review_decision_gate(
619,
"approve",
final_review_decision_ready=True,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertEqual(reasons, [], reasons)
def test_submit_gate_blocks_same_head_duplicate(self):
mutations = [RC_619_HEAD_A]
_seed(
mutations,
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
lock = mcp_server._load_review_decision_lock()
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = 619
lock["ready_action"] = "request_changes"
lock["ready_expected_head_sha"] = HEAD_A
lock["ready_remote"] = "prgs"
lock["ready_org"] = "Scaled-Tech-Consulting"
lock["ready_repo"] = "Gitea-Tools"
mcp_server._save_review_decision_lock(lock)
reasons = mcp_server.check_review_decision_gate(
619,
"request_changes",
final_review_decision_ready=True,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(reasons)
class TestAssessmentOpenPrHead(unittest.TestCase):
def test_open_pr_stale_by_head_no_cleanup(self):
lock = _lock(
[RC_619_HEAD_A],
ready_pr=619,
ready_head=HEAD_A,
ready_action="request_changes",
)
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(619, HEAD_B)
)
self.assertTrue(a["has_lock"])
self.assertFalse(a["is_moot"])
self.assertFalse(a["cleanup_allowed"])
self.assertTrue(a["stale_by_head"])
self.assertTrue(a["fresh_review_on_current_head_allowed"])
self.assertEqual(a["locked_head_sha"], HEAD_A)
self.assertEqual(a["current_pr_head_sha"], HEAD_B)
def test_open_pr_same_head_no_fresh(self):
lock = _lock([RC_619_HEAD_A], ready_pr=619, ready_head=HEAD_A)
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(619, HEAD_A)
)
self.assertFalse(a["stale_by_head"])
self.assertFalse(a["fresh_review_on_current_head_allowed"])
self.assertFalse(a["cleanup_allowed"])
def test_historical_mutation_preserved_in_summary(self):
lock = _lock(
[RC_619_HEAD_A, APPROVE_619_HEAD_B],
ready_pr=619,
ready_head=HEAD_B,
ready_action="approve",
)
summary = srdl.lock_summary(lock)
self.assertEqual(summary["live_mutations_count"], 2)
self.assertEqual(summary["last_terminal"]["head_sha"], HEAD_B)
self.assertEqual(summary["locked_head_sha"], HEAD_B)
if __name__ == "__main__":
unittest.main()
+195
View File
@@ -0,0 +1,195 @@
"""Tests for the pre-create issue content gate (Issue #582)."""
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
from issue_content_gate import ( # noqa: E402
assess_issue_content,
find_vague_reference,
has_durable_source_pointer,
normalize_text,
pre_create_issue_content_gate,
)
from mcp_server import gitea_create_issue # noqa: E402
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
ISSUE_WRITE_ENV = {
"GITEA_ALLOWED_OPERATIONS": "gitea.issue.create",
}
class TestNormalizeAndPointers(unittest.TestCase):
def test_normalize_strips_punctuation_and_case(self):
self.assertEqual(normalize_text(" The Drafted-Issue! "), "the drafted issue")
def test_issue_ref_is_durable_pointer(self):
self.assertTrue(has_durable_source_pointer("See #402 for the spec"))
def test_file_path_is_durable_pointer(self):
self.assertTrue(has_durable_source_pointer("Draft in task_capability_map.py"))
def test_scratchpad_is_durable_pointer(self):
self.assertTrue(has_durable_source_pointer("Full text in scratchpad/draft.md"))
def test_url_is_durable_pointer(self):
self.assertTrue(
has_durable_source_pointer("https://gitea.prgs.cc/issues/1 has it")
)
def test_plain_text_has_no_durable_pointer(self):
self.assertFalse(has_durable_source_pointer("just some words here"))
class TestFindVagueReference(unittest.TestCase):
def test_bare_vague_phrases_are_flagged(self):
for text in (
"the drafted issue",
"The prepared issue",
"the issue we discussed",
"the previous draft",
"Create the drafted issue",
"please file the prepared issue",
):
with self.subTest(text=text):
self.assertIsNotNone(find_vague_reference(text))
def test_durable_pointer_defeats_vague_reference(self):
# Same vague phrase but with a durable pointer -> not vague.
self.assertIsNone(find_vague_reference("the drafted issue in #402"))
self.assertIsNone(
find_vague_reference("the prepared issue: see scratchpad/draft.md")
)
def test_long_specific_body_with_incidental_phrase_is_not_vague(self):
body = (
"As discussed, the create_issue tool must reject vague references. "
"Add a preflight check that lists the exact missing fields and "
"returns BLOCKED + DIAGNOSE with concrete acceptance criteria."
)
self.assertIsNone(find_vague_reference(body))
def test_concrete_short_body_is_not_vague(self):
self.assertIsNone(find_vague_reference("Fix null deref in auth handler"))
def test_empty_is_not_vague(self):
self.assertIsNone(find_vague_reference(""))
class TestAssessIssueContent(unittest.TestCase):
def test_missing_title_is_incomplete(self):
result = assess_issue_content("", "some body")
self.assertFalse(result["complete"])
self.assertIn("title", result["missing_fields"])
def test_vague_body_is_incomplete_and_diagnosed(self):
result = assess_issue_content("Add the thing", "the drafted issue")
self.assertFalse(result["complete"])
self.assertIn("body", result["vague_fields"])
self.assertIn("vague reference", result["diagnose"])
def test_vague_title_is_incomplete(self):
result = assess_issue_content("the prepared issue", "")
self.assertFalse(result["complete"])
self.assertIn("title", result["vague_fields"])
def test_complete_content_passes(self):
result = assess_issue_content(
"Block vague issue creation",
"Add a preflight gate that rejects placeholder references.",
)
self.assertTrue(result["complete"])
self.assertEqual(result["missing_fields"], [])
self.assertEqual(result["vague_fields"], [])
def test_title_only_is_allowed(self):
# Empty body must NOT block: title-only creation stays allowed.
result = assess_issue_content("A concrete specific title", "")
self.assertTrue(result["complete"])
def test_durable_pointer_body_passes(self):
result = assess_issue_content(
"Implement the drafted change",
"the drafted issue is fully specified in #582",
)
self.assertTrue(result["complete"])
def test_allow_incomplete_override(self):
result = assess_issue_content(
"the drafted issue", "the drafted issue", allow_incomplete=True
)
self.assertTrue(result["complete"])
self.assertTrue(result["override_applied"])
def test_gate_wrapper_sets_performed(self):
blocked = pre_create_issue_content_gate("Add x", "the drafted issue")
self.assertFalse(blocked["performed"])
ok = pre_create_issue_content_gate("Add x", "concrete actionable body")
self.assertTrue(ok["performed"])
class TestCreateIssueContentMCPGate(unittest.TestCase):
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop")
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_blocks_vague_reference_without_creating(
self, _auth, mock_get_all, mock_api, mock_role_check
):
mock_role_check.return_value = (True, [])
with patch.dict(__import__("os").environ, ISSUE_WRITE_ENV, clear=True):
result = gitea_create_issue(
title="Create the drafted issue",
body="the drafted issue",
remote="prgs",
)
self.assertFalse(result["performed"])
self.assertIn("content_gate", result)
self.assertIn("body", result["content_gate"]["vague_fields"])
self.assertTrue(any("BLOCKED + DIAGNOSE" in r for r in result["reasons"]))
mock_api.assert_not_called()
mock_get_all.assert_not_called()
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop")
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_creates_when_content_is_concrete(
self, _auth, _get_all, mock_api, mock_role_check
):
mock_role_check.return_value = (True, [])
mock_api.return_value = {"number": 7, "html_url": "https://x/issues/7"}
with patch.dict(__import__("os").environ, ISSUE_WRITE_ENV, clear=True):
result = gitea_create_issue(
title="Block vague issue creation",
body="Add a preflight gate rejecting placeholder references.",
remote="prgs",
)
self.assertEqual(result["number"], 7)
@patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop")
@patch("mcp_server.api_request")
@patch("mcp_server.api_get_all", return_value=[])
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_override_creates_despite_vague_content(
self, _auth, _get_all, mock_api, mock_role_check
):
mock_role_check.return_value = (True, [])
mock_api.return_value = {"number": 8, "html_url": "https://x/issues/8"}
with patch.dict(__import__("os").environ, ISSUE_WRITE_ENV, clear=True):
result = gitea_create_issue(
title="the drafted issue",
body="the drafted issue",
remote="prgs",
allow_incomplete_content=True,
)
self.assertEqual(result["number"], 8)
if __name__ == "__main__":
unittest.main()
+208
View File
@@ -0,0 +1,208 @@
import unittest
import gitea_mcp_server
import mcp_namespace_health
import review_merge_state_machine as rmsm
class TestMcpNamespaceHealth(unittest.TestCase):
def setUp(self):
gitea_mcp_server._LIVE_NAMESPACE_HEALTH.clear()
def test_registered_tool_but_live_eof_blocks_merge(self):
result = mcp_namespace_health.classify_namespace_probe(
"gitea-reviewer",
required_tool="gitea_whoami",
registered_tools=["gitea_whoami", "gitea_list_profiles"],
probe_result={
"success": False,
"error": "client is closing: EOF",
},
process={
"pid": 4321,
"profile": "prgs-reviewer",
"env": {
"GITEA_MCP_PROFILE": "prgs-reviewer",
"GITEA_TOKEN": "must-not-leak",
},
},
config_path="/Users/jasonwalker/.gemini/config/mcp_config.json",
probe_source="client_namespace",
)
self.assertFalse(result["healthy"])
self.assertEqual(result["error_type"], "namespace_eof")
self.assertTrue(result["required_tool_registered"])
self.assertFalse(result["required_tool_callable"])
self.assertTrue(result["blocks_merge_workflow"])
self.assertFalse(result["ide_namespace_proven"])
self.assertEqual(result["probe_source"], "client_namespace")
self.assertEqual(result["diagnostics"]["process_pid"], 4321)
self.assertEqual(result["diagnostics"]["profile"], "prgs-reviewer")
self.assertEqual(
result["diagnostics"]["env"]["GITEA_MCP_PROFILE"],
"prgs-reviewer",
)
self.assertNotIn("GITEA_TOKEN", result["diagnostics"]["env"])
self.assertIn("gitea-reviewer", result["reasons"][0])
self.assertIn("gitea_whoami", result["reasons"][0])
def test_successful_client_namespace_invocation_is_ide_proven(self):
result = mcp_namespace_health.classify_namespace_probe(
"gitea-author",
registered_tools=["gitea_whoami"],
probe_result={"success": True, "result": {"authenticated": True}},
process={"pid": 1234, "profile": "prgs-author"},
config_path="/tmp/mcp_config.json",
probe_source="client_namespace",
)
self.assertTrue(result["healthy"])
self.assertTrue(result["required_tool_registered"])
self.assertTrue(result["required_tool_callable"])
self.assertTrue(result["ide_namespace_proven"])
self.assertFalse(result["blocks_merge_workflow"])
self.assertIsNone(result["error_type"])
def test_offline_spawn_success_is_not_ide_proof(self):
result = mcp_namespace_health.classify_namespace_probe(
"gitea-merger",
registered_tools=["gitea_whoami"],
probe_result={"success": True, "result": {}},
process={"pid": 99, "profile": "prgs-merger"},
probe_source="offline_spawn",
)
self.assertTrue(result["healthy"])
self.assertFalse(result["ide_namespace_proven"])
self.assertFalse(result["blocks_merge_workflow"])
self.assertTrue(
any("offline_spawn" in r for r in result["reasons"])
)
def test_registered_missing_required_tool_fails_before_probe_success(self):
result = mcp_namespace_health.classify_namespace_probe(
"gitea-tools",
registered_tools=["gitea_whoami"],
probe_result={"success": True, "result": {}},
probe_source="client_namespace",
)
self.assertFalse(result["healthy"])
self.assertEqual(result["required_tool"], "gitea_list_profiles")
self.assertFalse(result["required_tool_registered"])
self.assertEqual(result["error_type"], "tool_missing")
def test_server_tool_exposes_same_assessment_and_records_session(self):
result = gitea_mcp_server.gitea_assess_mcp_namespace_health(
"gitea-merger",
registered_tools=["gitea_whoami"],
probe_result={"success": False, "error": "transport closed"},
process={"pid": 9876, "profile": "prgs-merger"},
probe_source="client_namespace",
)
self.assertFalse(result["success"])
self.assertEqual(result["error_type"], "namespace_eof")
self.assertEqual(result["namespace"], "gitea-merger")
self.assertEqual(result["diagnostics"]["process_pid"], 9876)
self.assertIn("gitea-merger", gitea_mcp_server._LIVE_NAMESPACE_HEALTH)
gate = gitea_mcp_server._live_namespace_health_gate("merge_pr")
self.assertTrue(gate)
self.assertTrue(any("gitea-merger" in r for r in gate))
class TestLiveNamespaceBlocksMerge(unittest.TestCase):
"""AC5: a broken live namespace must hard-block the review/merge state machine."""
def setUp(self):
gitea_mcp_server._LIVE_NAMESPACE_HEALTH.clear()
def _merge_ready_completion(self):
# Completion through PRE_MERGE_RECHECK is the state where a clean merge
# is otherwise allowed (see test_merge_allowed_with_pre_merge_gates).
idx = rmsm.REVIEW_MERGE_STATES.index("PRE_MERGE_RECHECK")
return {state: True for state in rmsm.REVIEW_MERGE_STATES[: idx + 1]}
def _all_gates(self):
return {gate: True for gate in rmsm._PRE_MERGE_REQUIRED_GATES}
def test_assess_workflow_blockers_flags_live_namespace_broken(self):
clean = rmsm.assess_workflow_blockers()
self.assertFalse(clean["block"])
broken = rmsm.assess_workflow_blockers(live_namespace_broken=True)
self.assertTrue(broken["block"])
self.assertTrue(any("namespace" in r.lower() for r in broken["reasons"]))
def test_can_merge_blocks_even_when_all_gates_pass(self):
# Without the namespace blocker a fully-complete workflow can merge...
allowed = rmsm.can_merge(
self._merge_ready_completion(), pre_merge_gates=self._all_gates()
)
self.assertTrue(allowed["allowed"])
# ...but a broken live namespace overrides every satisfied gate.
blocked = rmsm.can_merge(
self._merge_ready_completion(),
pre_merge_gates=self._all_gates(),
live_namespace_broken=True,
)
self.assertTrue(blocked["block"])
self.assertFalse(blocked["allowed"])
def test_workflow_status_forwards_namespace_blocker(self):
status = rmsm.workflow_status(
self._merge_ready_completion(), live_namespace_broken=True
)
self.assertFalse(status["merge_allowed"])
self.assertFalse(status["approve_allowed"])
def test_server_tool_blocks_merge_on_live_namespace_broken(self):
result = gitea_mcp_server.gitea_assess_review_merge_state_machine(
state_completion=self._merge_ready_completion(),
pre_merge_gates=self._all_gates(),
live_namespace_broken=True,
)
self.assertTrue(result["blockers"]["block"])
self.assertFalse(result["merge"]["allowed"])
def test_classify_verdict_bridges_into_merge_block(self):
# The classify verdict is the intended feed for live_namespace_broken.
verdict = mcp_namespace_health.classify_namespace_probe(
"gitea-merger",
registered_tools=["gitea_whoami", "gitea_adopt_merger_pr_lease"],
probe_result={"success": False, "error": "client is closing: EOF"},
process={"pid": 555, "profile": "prgs-merger"},
probe_source="client_namespace",
)
self.assertTrue(verdict["blocks_merge_workflow"])
blocked = rmsm.can_merge(
self._merge_ready_completion(),
pre_merge_gates=self._all_gates(),
live_namespace_broken=verdict["blocks_merge_workflow"],
)
self.assertFalse(blocked["allowed"])
def test_offline_spawn_does_not_authorize_mutation_gate(self):
gitea_mcp_server.gitea_assess_mcp_namespace_health(
"gitea-merger",
registered_tools=["gitea_whoami"],
probe_result={"success": True, "result": {}},
probe_source="offline_spawn",
)
gate = gitea_mcp_server._live_namespace_health_gate("merge_pr")
self.assertTrue(gate)
self.assertTrue(any("offline_spawn" in r or "client_namespace" in r for r in gate))
def test_client_namespace_healthy_clears_mutation_gate(self):
gitea_mcp_server.gitea_assess_mcp_namespace_health(
"gitea-merger",
registered_tools=["gitea_whoami"],
probe_result={"success": True, "result": {}},
probe_source="client_namespace",
)
self.assertEqual(gitea_mcp_server._live_namespace_health_gate("merge_pr"), [])
if __name__ == "__main__":
unittest.main()
+111
View File
@@ -774,6 +774,9 @@ class TestMergePR(unittest.TestCase):
def setUp(self): def setUp(self):
import reviewer_pr_lease import reviewer_pr_lease
# Clean ledger each merge test so residual/host durable #332 state
# cannot short-circuit eligibility gates (#590 / #594).
mcp_server._save_review_decision_lock(None)
mcp_server.gitea_load_review_workflow() mcp_server.gitea_load_review_workflow()
self._lease_patch = _install_owned_reviewer_lease(8) self._lease_patch = _install_owned_reviewer_lease(8)
self._lease_patch.start() self._lease_patch.start()
@@ -1003,6 +1006,64 @@ class TestMergePR(unittest.TestCase):
r["reasons"]) r["reasons"])
self._assert_no_merge_call(mock_api) self._assert_no_merge_call(mock_api)
@patch("mcp_server.api_request")
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_env_clear_does_not_adopt_host_review_mutation_ledger(
self, _auth, mock_api
):
"""#590: patch.dict(clear=True) must not load host terminal mutations.
Simulates a dirty host session-state file for profile gitea-default
(the profile active when env is fully cleared). Isolation must keep
the #332 hard-stop ledger clean so the empty-profile gate fires.
"""
import tempfile
import mcp_session_state
poison_dir = tempfile.mkdtemp(prefix="gitea-host-poison-")
mcp_session_state.save_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity="gitea-default",
state_dir=poison_dir,
payload={
"task": "review_pr",
"remote": "prgs",
"final_review_decision_ready": True,
"ready_pr_number": 8,
"ready_action": "request_changes",
"live_mutations": [
{
"pr_number": 8,
"action": "request_changes",
"review_id": 99,
"review_state": "request_changes",
}
],
},
)
# If isolation only used the env var, clear=True would fall back to
# DEFAULT_STATE_DIR and adopt this poison ledger.
mcp_server._REVIEW_DECISION_LOCK = None
mock_api.side_effect = [{"login": "merger-bot"}, self._pr("author-bot")]
with patch.object(mcp_session_state, "DEFAULT_STATE_DIR", poison_dir):
with patch.dict(os.environ, {}, clear=True):
r = gitea_merge_pr(
pr_number=8,
confirmation=self._confirm(8),
remote="prgs",
)
self.assertFalse(r["performed"])
self.assertIn(
"profile has no configured allowed operations (fail closed)",
r["reasons"],
)
self.assertFalse(
any("terminal review mutation already consumed" in x for x in r["reasons"]),
msg=f"host ledger leaked into reasons: {r['reasons']}",
)
self._assert_no_merge_call(mock_api)
@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_profile_without_merge_permission_blocks(self, _auth, mock_api): def test_profile_without_merge_permission_blocks(self, _auth, mock_api):
@@ -2672,6 +2733,56 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
self.assertTrue(res["success"]) self.assertTrue(res["success"])
self.assertEqual(res["cleanup_status"].get(1), "not present") self.assertEqual(res["cleanup_status"].get(1), "not present")
def test_close_issue_blocks_unproven_expected_preexisting_failure(self):
# #529: a closure report calling a non-zero suite exit an "expected
# pre-existing failure" without pre-merge proof must fail closed and
# never PATCH the issue state.
patched = []
def api_side_effect(method, url, auth, payload=None):
if method == "PATCH":
patched.append(url)
return {}
self.mock_api.side_effect = api_side_effect
res = gitea_close_issue(
issue_number=1,
closure_report=(
"Full suite: 1 failed. Expected pre-existing failure, safe to close."
),
)
self.assertFalse(res["success"])
self.assertTrue(res.get("blocked"))
self.assertTrue(res.get("reasons"))
self.assertFalse(
patched, "must not PATCH issue state when the closure gate blocks"
)
def test_close_issue_allows_proven_baseline_closure(self):
def api_side_effect(method, url, auth, payload=None):
if method == "PATCH" and "issues/1" in url:
return {"state": "closed"}
if method == "GET" and "labels" in url and "issues" not in url:
return [{"name": "bug", "id": 2}]
if method == "GET" and "issues/1" in url:
return {"labels": [{"name": "bug"}]}
return {}
self.mock_api.side_effect = api_side_effect
res = gitea_close_issue(
issue_number=1,
closure_report=(
"Full suite: 1 failed, expected pre-existing baseline failure "
"proven on the PR pre-merge base commit.\n"
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
"Command: venv/bin/python -m pytest -q\n"
"Exit status: 1\n"
"Failure signature: AssertionError: None is not true\n"
),
)
self.assertTrue(res["success"])
def test_merge_pr_with_closes_removes_label(self): def test_merge_pr_with_closes_removes_label(self):
import reviewer_pr_lease import reviewer_pr_lease
+61
View File
@@ -67,5 +67,66 @@ class TestMcpStaleRuntime(unittest.TestCase):
# But does NOT contain missing author profile error # But does NOT contain missing author profile error
self.assertFalse(any("None of the matching profiles for task" in r for r in reasons_author)) self.assertFalse(any("None of the matching profiles for task" in r for r in reasons_author))
@patch("subprocess.run")
@patch("os.path.getmtime")
@patch("os.path.exists")
@patch("os.getpid")
@patch.dict("os.environ", {"GITEA_FORCE_MCP_RUNTIME_CHECK": "1"})
def test_git_head_change_triggers_staleness(self, mock_getpid, mock_exists, mock_getmtime, mock_run):
# Set boot SHA to FAKE1 and current to FAKE2
gitea_mcp_server._process_boot_head_sha = "FAKE1"
mock_getpid.return_value = 12345
mock_exists.return_value = True
# Code time is in the past (fresh process chronologically)
code_time = datetime(2026, 7, 8, 11, 0, 0)
mock_getmtime.return_value = code_time.timestamp()
ps_output = (
" PID LSTART COMMAND\n"
"12345 Wed Jul 8 12:00:00 2026 /path/to/python mcp_server.py\n"
)
mock_run_ps = MagicMock()
mock_run_ps.stdout = ps_output
mock_run_env = MagicMock()
mock_run_env.stdout = "GITEA_MCP_PROFILE=prgs-author"
mock_run_git = MagicMock()
mock_run_git.stdout = "FAKE2" # different SHA
def side_effect(args, **kwargs):
if args[0] == "ps" and "eww" in args:
return mock_run_env
elif args[0] == "ps":
return mock_run_ps
elif args[0] == "git" and "rev-parse" in args:
return mock_run_git
raise ValueError(f"Unexpected: {args}")
mock_run.side_effect = side_effect
# Stale even though process started AFTER code mtime, because git HEAD changed
reasons = gitea_mcp_server._check_mcp_runtimes_diagnostics("create_issue", ["prgs-author"])
self.assertTrue(any("stale-runtime: The active Gitea MCP server process is stale" in r for r in reasons))
@patch("threading.Thread")
@patch("os.utime")
@patch("os.path.exists")
@patch.dict("os.environ", {"MCP_CONFIG_PATH": "/tmp/mcp_config.json"})
def test_auto_restart_trigger_touches_and_spawns(self, mock_exists, mock_utime, mock_thread):
mock_exists.return_value = True
gitea_mcp_server._restart_triggered = False
# Ensure we are not skipped in test mode for testing purposes
with patch("gitea_mcp_server._preflight_in_test_mode", return_value=False):
gitea_mcp_server._trigger_mcp_auto_restart()
mock_utime.assert_called_once_with("/tmp/mcp_config.json", None)
mock_thread.assert_called_once()
self.assertTrue(gitea_mcp_server._restart_triggered)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+90
View File
@@ -42,6 +42,96 @@ def _lease_comment(
HEAD = "f" * 40 HEAD = "f" * 40
class TestDescribeSessionLeaseProof(unittest.TestCase):
def setUp(self):
leases.clear_session_lease()
def test_empty_session_is_none_kind(self):
proof = mla.describe_session_lease_proof(None)
self.assertIsNone(proof["lease_proof_source"])
self.assertEqual(proof["lease_proof_kind"], "none")
self.assertFalse(proof["lease_proof_sanctioned"])
def test_manual_seed_without_provenance(self):
leases.record_session_lease({
"pr_number": 536,
"session_id": "manual",
"comment_id": 1,
})
proof = mla.describe_session_lease_proof(leases.get_session_lease())
self.assertIsNone(proof["lease_proof_source"])
self.assertEqual(proof["lease_proof_kind"], "unsanctioned_manual_seed")
self.assertFalse(proof["lease_proof_sanctioned"])
def test_sanctioned_adopt_fields(self):
leases.record_session_lease(
{
"pr_number": 536,
"session_id": "merger-sess",
"comment_id": 700,
},
lease_provenance=mla.build_lease_provenance(
source=mla.SOURCE_ADOPT,
comment_id=700,
adopted_from_session_id="reviewer-sess",
adoption_reason=mla.DEFAULT_ADOPTION_REASON,
),
)
proof = mla.describe_session_lease_proof(leases.get_session_lease())
self.assertEqual(proof["lease_proof_source"], mla.SOURCE_ADOPT)
self.assertEqual(proof["lease_proof_kind"], "sanctioned_adoption")
self.assertTrue(proof["lease_proof_sanctioned"])
self.assertEqual(
proof["lease_recovery_reason"], mla.DEFAULT_ADOPTION_REASON
)
self.assertEqual(proof["lease_proof_comment_id"], 700)
self.assertEqual(proof["lease_adopted_from_session_id"], "reviewer-sess")
def test_sanctioned_acquire_fields(self):
leases.record_session_lease(
{
"pr_number": 536,
"session_id": "rev",
"comment_id": 50,
},
lease_provenance=mla.build_lease_provenance(
source=mla.SOURCE_ACQUIRE,
comment_id=50,
),
)
proof = mla.describe_session_lease_proof(leases.get_session_lease())
self.assertEqual(proof["lease_proof_source"], mla.SOURCE_ACQUIRE)
self.assertEqual(proof["lease_proof_kind"], "sanctioned_acquire")
self.assertTrue(proof["lease_proof_sanctioned"])
class TestManualLeaseProofHandoffAudit(unittest.TestCase):
def test_clean_merge_report_passes(self):
text = (
"Merged via gitea_merge_pr after gitea_adopt_merger_pr_lease. "
"lease_proof_source: gitea_adopt_merger_pr_lease"
)
result = mla.assess_manual_lease_proof_handoff(text)
self.assertTrue(result["proven"])
self.assertFalse(result["block"])
def test_manual_seeded_claim_without_mcp_evidence_blocks(self):
text = "Merged using manually seeded lease proof from prior session."
result = mla.assess_manual_lease_proof_handoff(text)
self.assertFalse(result["proven"])
self.assertTrue(result["block"])
self.assertTrue(any("#535" in r for r in result["reasons"]))
def test_historical_manual_mention_allowed_with_adoption_evidence(self):
text = (
"Historical incident used manually seeded lease; this merge used "
"gitea_adopt_merger_pr_lease with adoption_comment_id: 8288."
)
result = mla.assess_manual_lease_proof_handoff(text)
self.assertTrue(result["proven"])
self.assertFalse(result["block"])
class TestSanctionedProvenance(unittest.TestCase): class TestSanctionedProvenance(unittest.TestCase):
def setUp(self): def setUp(self):
leases.clear_session_lease() leases.clear_session_lease()
+15
View File
@@ -88,6 +88,21 @@ class TestControlPlaneGuide(GuideTestBase):
blob = " ".join(g["guidance"]).lower() blob = " ".join(g["guidance"]).lower()
self.assertIn("forbidden", blob) self.assertIn("forbidden", blob)
self.assertIn("review", blob) self.assertIn("review", blob)
self.assertIn("resolved_repository", g)
@patch("mcp_server.api_request", return_value={"login": "author-bot"})
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_guide_explicit_org_repo_parameters(self, _auth, _api):
with patch.dict(os.environ, AUTHOR_ENV, clear=True):
g = mcp_get_control_plane_guide(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertEqual(
g["resolved_repository"],
{"org": "Scaled-Tech-Consulting", "repo": "Gitea-Tools"},
)
@patch("mcp_server.api_request", return_value={"login": "reviewer-bot"}) @patch("mcp_server.api_request", return_value={"login": "reviewer-bot"})
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
+119
View File
@@ -0,0 +1,119 @@
"""Post-merge moot validation wording + label tests (#529, criteria 1/4/5)."""
import unittest
from post_merge_validation import (
BASELINE_ACCEPTED_OUTCOME,
BLOCKED_OUTCOME,
CLEAN_PASS_OUTCOME,
LABEL_BASELINE_ACCEPTED,
LABEL_BLOCKED,
LABEL_CLEAN_PASS,
LABEL_POST_MERGE_MOOT,
POST_MERGE_MOOT_OUTCOME,
assess_post_merge_validation,
process_state_label,
)
from final_report_validator import assess_final_report_validator
from issue_workflow_labels import CANONICAL_LABELS, VALIDATION_LABELS
_MOOT_PROOF = (
"Validation status: post-merge moot validation\n"
"PR state: merged\n"
"Merge commit sha: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
"Validation finding: full suite passed, for the record.\n"
)
class TestPostMergeValidation(unittest.TestCase):
def test_not_applicable_when_no_merged_or_moot_signal(self):
result = assess_post_merge_validation(
"Review decision: approve. Validation: full suite passed."
)
self.assertFalse(result["block"])
self.assertTrue(result["skipped"])
def test_active_approval_on_merged_pr_blocked(self):
report = (
"PR is already merged. Review decision: approve. "
"Validation: full suite passed."
)
result = assess_post_merge_validation(report)
self.assertTrue(result["block"])
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
self.assertEqual(result["process_state_label"], LABEL_POST_MERGE_MOOT)
def test_active_approval_on_merged_pr_via_structured_signal_blocked(self):
report = "Review decision: approve. Validation: full suite passed."
result = assess_post_merge_validation(report, pr_merged_or_closed=True)
self.assertTrue(result["block"])
def test_moot_wording_without_proof_blocked(self):
report = "Recording post-merge moot validation for this PR."
result = assess_post_merge_validation(report)
self.assertTrue(result["block"])
self.assertTrue(result["reasons"])
def test_moot_wording_with_full_proof_allowed(self):
result = assess_post_merge_validation(_MOOT_PROOF)
self.assertFalse(result["block"])
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
def test_merged_signal_only_guides_without_blocking(self):
result = assess_post_merge_validation(
"PR was already merged before review; recording findings."
)
self.assertFalse(result["block"])
self.assertTrue(result["safe_next_action"])
def test_process_state_label_mapping(self):
self.assertEqual(process_state_label(CLEAN_PASS_OUTCOME), LABEL_CLEAN_PASS)
self.assertEqual(
process_state_label(BASELINE_ACCEPTED_OUTCOME), LABEL_BASELINE_ACCEPTED
)
self.assertEqual(process_state_label(BLOCKED_OUTCOME), LABEL_BLOCKED)
self.assertEqual(
process_state_label(POST_MERGE_MOOT_OUTCOME), LABEL_POST_MERGE_MOOT
)
self.assertIsNone(process_state_label("nonsense"))
class TestValidationLabelsRegistered(unittest.TestCase):
def test_validation_labels_are_canonical(self):
for label in (
LABEL_CLEAN_PASS,
LABEL_BASELINE_ACCEPTED,
LABEL_BLOCKED,
LABEL_POST_MERGE_MOOT,
):
self.assertIn(label, VALIDATION_LABELS)
self.assertIn(label, CANONICAL_LABELS)
class TestPostMergeValidationWiredIntoReview(unittest.TestCase):
def test_review_pr_blocks_active_approval_on_merged_pr(self):
report = (
"PR is already merged. Review decision: approve. "
"Validation: full suite passed."
)
result = assess_final_report_validator(report, "review_pr")
self.assertTrue(result["blocked"])
self.assertTrue(
any(
f["rule_id"] == "reviewer.post_merge_validation"
for f in result["findings"]
)
)
def test_review_pr_allows_proven_post_merge_moot(self):
result = assess_final_report_validator(_MOOT_PROOF, "review_pr")
self.assertFalse(
any(
f["rule_id"] == "reviewer.post_merge_validation"
for f in result["findings"]
)
)
if __name__ == "__main__":
unittest.main()
+245
View File
@@ -0,0 +1,245 @@
"""Issue #525: reconciler supersession close path."""
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import mcp_server # noqa: E402
import role_session_router # noqa: E402
import task_capability_map # noqa: E402
from review_proofs import assess_reconciler_supersession_close_gate # noqa: E402
HEAD = "0fdc8f582026b72a229d59a172c0a63ac4aaeaf9"
MERGE = "1111111111111111111111111111111111111111"
TARGET = "2222222222222222222222222222222222222222"
def _target_pr(mergeable=False):
return {
"number": 490,
"state": "open",
"mergeable": mergeable,
"head": {"sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
}
def _superseding_pr():
return {
"number": 502,
"state": "closed",
"merged": True,
"merge_commit_sha": MERGE,
"head": {"sha": HEAD},
}
class TestReconcilerSupersessionCapabilityMap(unittest.TestCase):
def test_supersession_tasks_route_to_reconciler(self):
self.assertEqual(
task_capability_map.required_permission(
"reconcile_close_superseded_pr"),
"gitea.pr.close",
)
self.assertEqual(
task_capability_map.required_role("reconcile_close_superseded_pr"),
"reconciler",
)
self.assertEqual(
task_capability_map.required_permission(
"reconcile_close_satisfied_issue"),
"gitea.issue.close",
)
self.assertEqual(
task_capability_map.required_role("reconcile_create_followup_issue"),
"reconciler",
)
def test_author_session_cannot_route_supersession_close(self):
result = role_session_router.route_task_session(
"reconcile_close_superseded_pr",
active_profile="prgs-author",
active_role_kind="author",
allowed_in_current_session=False,
)
self.assertEqual(result["route_result"], "wrong_role_stop")
self.assertFalse(result["downstream_allowed"])
class TestReconcilerSupersessionCloseGate(unittest.TestCase):
def _gate(self, **overrides):
kwargs = {
"target_pr_number": 490,
"target_pr_state": "open",
"target_pr_mergeable": False,
"target_independently_required": False,
"superseding_pr_number": 502,
"superseding_pr_state": "closed",
"superseding_pr_merged": True,
"superseding_merge_commit_sha": MERGE,
"superseding_head_sha": HEAD,
"target_branch": "master",
"target_branch_sha": TARGET,
"superseding_head_is_ancestor_of_target": True,
"canonical_comment_valid": True,
"canonical_comment_mentions_superseding_pr": True,
"canonical_comment_mentions_merge_commit": True,
"close_pr_capability": True,
"close_issue_capability": True,
"target_issue_state": "open",
"issue_satisfied_by_superseding": True,
}
kwargs.update(overrides)
return assess_reconciler_supersession_close_gate(**kwargs)
def test_supersession_close_allowed_with_full_proof(self):
result = self._gate()
self.assertEqual(result["outcome"], "SUPERSESSION_CLOSE_ALLOWED")
self.assertTrue(result["pr_close_allowed"])
self.assertTrue(result["issue_close_allowed"])
self.assertFalse(result["review_allowed"])
self.assertFalse(result["merge_allowed"])
def test_mergeable_target_still_blocks(self):
result = self._gate(target_pr_mergeable=True)
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
self.assertFalse(result["pr_close_allowed"])
self.assertTrue(any("mergeable" in r for r in result["reasons"]))
def test_requires_canonical_comment_with_merge_commit(self):
result = self._gate(canonical_comment_mentions_merge_commit=False)
self.assertEqual(result["outcome"], "GATE_NOT_PROVEN")
self.assertFalse(result["pr_close_allowed"])
self.assertTrue(any("merge commit" in r for r in result["reasons"]))
def test_superseding_head_must_be_on_target_branch(self):
result = self._gate(superseding_head_is_ancestor_of_target=False)
self.assertEqual(result["outcome"], "SUPERSEDING_PR_NOT_ON_TARGET")
self.assertFalse(result["pr_close_allowed"])
def test_close_capability_required(self):
result = self._gate(close_pr_capability=False)
self.assertEqual(result["outcome"], "RECOVERY_HANDOFF_REQUIRED")
self.assertFalse(result["pr_close_allowed"])
class TestReconcilerSupersessionMcpTool(unittest.TestCase):
def _comment(self):
return f"""## Canonical PR State
STATE: superseded
NEXT_ACTION: Close superseded PR #490 and issue #470; PR #502 is canonical.
NEXT_ACTOR: prgs-reconciler
SUMMARY: PR #490 is superseded by merged PR #502 at merge commit {MERGE}.
CANONICAL_ITEM: PR #502
SUPERSEDED_ITEM: PR #490
CLOSE_OR_KEEP_OPEN: close superseded PR #490 and satisfied issue #470
"""
@patch("mcp_server.verify_preflight_purity")
@patch("mcp_server._canonical_comment_gate")
@patch("already_landed_reconcile.is_head_ancestor_of_ref", return_value=True)
@patch("already_landed_reconcile.fetch_target_branch")
@patch("mcp_server._profile_operation_gate", return_value=[])
@patch("mcp_server._auth", return_value="token test")
@patch("mcp_server.api_request")
def test_tool_posts_comment_and_closes_superseded_pr_issue(
self,
api,
_auth,
_profile_gate,
fetch_target,
_ancestor,
canonical_gate,
verify_preflight,
):
fetch_target.return_value = {
"success": True,
"target_branch": "master",
"target_ref": "prgs/master",
"target_branch_sha": TARGET,
}
canonical_gate.return_value = {
"blocked": False,
"canonical_comment_validation": {"valid": True},
}
api.side_effect = [
_target_pr(),
_superseding_pr(),
{"number": 470, "state": "open"},
{"id": 123},
{"state": "closed"},
{"state": "closed"},
]
result = mcp_server.gitea_reconcile_superseded_by_merged_pr(
target_pr_number=490,
superseding_pr_number=502,
target_issue_number=470,
target_independently_required=False,
issue_satisfied_by_superseding=True,
close_pr=True,
close_issue=True,
post_comment=True,
comment_body=self._comment(),
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(result["success"])
self.assertTrue(result["performed"])
self.assertTrue(result["pr_comment_posted"])
self.assertTrue(result["pr_closed"])
self.assertTrue(result["issue_closed"])
verify_preflight.assert_called_once_with(
"prgs", task="reconcile_close_superseded_pr")
@patch("mcp_server.verify_preflight_purity")
@patch("mcp_server._canonical_comment_gate")
@patch("already_landed_reconcile.is_head_ancestor_of_ref", return_value=True)
@patch("already_landed_reconcile.fetch_target_branch")
@patch("mcp_server._profile_operation_gate", return_value=[])
@patch("mcp_server._auth", return_value="token test")
@patch("mcp_server.api_request")
def test_tool_does_not_mutate_when_target_still_mergeable(
self,
api,
_auth,
_profile_gate,
fetch_target,
_ancestor,
canonical_gate,
verify_preflight,
):
fetch_target.return_value = {
"success": True,
"target_branch": "master",
"target_ref": "prgs/master",
"target_branch_sha": TARGET,
}
canonical_gate.return_value = {
"blocked": False,
"canonical_comment_validation": {"valid": True},
}
api.side_effect = [_target_pr(mergeable=True), _superseding_pr()]
result = mcp_server.gitea_reconcile_superseded_by_merged_pr(
target_pr_number=490,
superseding_pr_number=502,
target_independently_required=False,
close_pr=True,
post_comment=True,
comment_body=self._comment(),
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertFalse(result["success"])
self.assertFalse(result["performed"])
self.assertFalse(result["pr_closed"])
self.assertFalse(result["pr_comment_posted"])
verify_preflight.assert_not_called()
self.assertEqual(api.call_count, 2)
+93
View File
@@ -111,6 +111,23 @@ class TestAssessRemoteRepoMatch(unittest.TestCase):
self.assertIn("Gitea-Tools", message) self.assertIn("Gitea-Tools", message)
self.assertIn("org=", message) self.assertIn("org=", message)
self.assertIn("repo=", message) self.assertIn("repo=", message)
# Recovery text must name a tool that actually accepts org/repo (#530 follow-up).
self.assertIn("mcp_get_control_plane_guide", message)
self.assertIn("schema does not list", message)
def test_parse_org_repo_from_https_url(self):
parsed = remote_repo_guard.parse_org_repo_from_remote_url(LOCAL_GITEA_TOOLS_URL)
self.assertEqual(parsed, ("Scaled-Tech-Consulting", "Gitea-Tools"))
def test_parse_org_repo_from_ssh_url(self):
parsed = remote_repo_guard.parse_org_repo_from_remote_url(
"[email protected]:Scaled-Tech-Consulting/Gitea-Tools.git"
)
self.assertEqual(parsed, ("Scaled-Tech-Consulting", "Gitea-Tools"))
def test_parse_org_repo_empty(self):
self.assertIsNone(remote_repo_guard.parse_org_repo_from_remote_url(None))
self.assertIsNone(remote_repo_guard.parse_org_repo_from_remote_url(""))
class TestResolveServerWiring(unittest.TestCase): class TestResolveServerWiring(unittest.TestCase):
@@ -173,6 +190,82 @@ class TestResolveServerWiring(unittest.TestCase):
with self.assertRaises(RuntimeError): with self.assertRaises(RuntimeError):
self.server.gitea_view_issue(issue_number=1, remote="prgs") self.server.gitea_view_issue(issue_number=1, remote="prgs")
def test_control_plane_guide_accepts_explicit_gitea_tools(self):
"""Guide schema accepts org/repo and resolves Gitea-Tools, not Timesheet."""
self._set_prgs_default_repo("Timesheet")
with mock.patch.object(
self.server, "api_request", return_value={"login": "guide-bot"}
), mock.patch.object(
self.server, "get_auth_header", return_value="Basic dGVzdA=="
), mock.patch.dict(
os.environ,
{
"GITEA_PROFILE_NAME": "author-test",
"GITEA_ALLOWED_OPERATIONS": "gitea.read",
"GITEA_FORBIDDEN_OPERATIONS": "gitea.pr.merge",
},
clear=False,
):
guide = self.server.mcp_get_control_plane_guide(
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(guide["read_only"])
self.assertEqual(
guide["resolved_repository"],
{"org": "Scaled-Tech-Consulting", "repo": "Gitea-Tools"},
)
self.assertNotEqual(guide["resolved_repository"]["repo"], "Timesheet")
def test_control_plane_guide_bare_remote_aligns_to_local_gitea_tools(self):
"""Bare remote=prgs in a Gitea-Tools worktree must not stay on Timesheet."""
self._set_prgs_default_repo("Timesheet")
with mock.patch.object(
self.server, "api_request", return_value={"login": "guide-bot"}
), mock.patch.object(
self.server, "get_auth_header", return_value="Basic dGVzdA=="
), mock.patch.dict(
os.environ,
{
"GITEA_PROFILE_NAME": "author-test",
"GITEA_ALLOWED_OPERATIONS": "gitea.read",
"GITEA_FORBIDDEN_OPERATIONS": "gitea.pr.merge",
},
clear=False,
):
guide = self.server.mcp_get_control_plane_guide(remote="prgs")
self.assertEqual(guide["resolved_repository"]["repo"], "Gitea-Tools")
self.assertEqual(
guide["resolved_repository"]["org"], "Scaled-Tech-Consulting"
)
def test_control_plane_guide_wrong_default_fails_closed_without_local(self):
"""Without local remote context, mismatched defaults still fail closed."""
self._set_prgs_default_repo("Timesheet")
with mock.patch.object(
self.server, "_local_git_remote_url", return_value=None
):
# No local URL → preference cannot recover; bare resolve uses Timesheet
# and guard is skipped only because local URL is missing (best-effort).
host, org, repo = self.server._resolve_control_plane_guide_target(
"prgs", None, None, None
)
self.assertEqual(repo, "Timesheet")
def test_control_plane_guide_schema_recovery_matches_parameters(self):
"""Remediation must only recommend parameters the guide actually accepts."""
import inspect
sig = inspect.signature(self.server.mcp_get_control_plane_guide)
self.assertIn("org", sig.parameters)
self.assertIn("repo", sig.parameters)
self.assertIn("remote", sig.parameters)
remediation = remote_repo_guard.REMEDIATION
self.assertIn("mcp_get_control_plane_guide", remediation)
self.assertIn("org=", remediation)
self.assertIn("repo=", remediation)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+43
View File
@@ -287,6 +287,49 @@ class TestResolveTaskCapability(unittest.TestCase):
self.assertEqual(res["required_operation_permission"], "gitea.pr.merge") self.assertEqual(res["required_operation_permission"], "gitea.pr.merge")
self.assertEqual(res["required_role_kind"], "merger") self.assertEqual(res["required_role_kind"], "merger")
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
def test_reviewer_role_with_merge_permission_still_cannot_merge(self, _auth, _api):
# #539: exact permission alone is insufficient. The active profile
# role must also be merger for merge_pr.
config = json.loads(json.dumps(CONFIG_RESOLVER))
config["profiles"]["reviewer-with-merge"] = {
"enabled": True,
"context": "ctx",
"role": "reviewer",
"username": "reviewer-user",
"auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"},
"allowed_operations": [
"gitea.read", "gitea.pr.review", "gitea.pr.approve",
"gitea.pr.merge",
],
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push"],
"execution_profile": "reviewer-with-merge",
}
self._write_config(config)
with patch.dict(os.environ, self._env("reviewer-with-merge")):
res = mcp_server.gitea_resolve_task_capability(
task="merge_pr", remote="prgs"
)
self.assertEqual(res["required_role_kind"], "merger")
self.assertEqual(res["active_role_kind"], "reviewer")
self.assertTrue(res["active_profile_permission_allowed"])
self.assertFalse(res["allowed_in_current_session"])
self.assertTrue(res["stop_required"])
self.assertIn("cannot perform merger task", " ".join(res["task_role_guidance"]))
@patch("mcp_server.api_request", return_value={"login": "merger-user"})
@patch("mcp_server.get_auth_header", return_value="token merger-pass")
def test_merger_profile_can_resolve_merge_task(self, _auth, _api):
with patch.dict(os.environ, self._env("merger-profile")):
res = mcp_server.gitea_resolve_task_capability(
task="merge_pr", remote="prgs"
)
self.assertEqual(res["required_role_kind"], "merger")
self.assertEqual(res["active_role_kind"], "merger")
self.assertTrue(res["allowed_in_current_session"])
self.assertFalse(res["stop_required"])
@patch("mcp_server.api_request") @patch("mcp_server.api_request")
def test_self_review_blocked_structured(self, mock_api): def test_self_review_blocked_structured(self, mock_api):
# Test that self-approve (self-review gate) is blocked and returns structured reasons # Test that self-approve (self-review gate) is blocked and returns structured reasons
+287
View File
@@ -0,0 +1,287 @@
"""Tests for preserving and resuming prepared Gitea review drafts (#609)."""
from __future__ import annotations
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch, MagicMock
sys_path_root = str(Path(__file__).resolve().parent.parent)
if sys_path_root not in sys.path:
sys.path.insert(0, sys_path_root)
import mcp_session_state
import gitea_mcp_server
import reviewer_pr_lease
class TestReviewDrafts(unittest.TestCase):
def setUp(self):
self._tmpdir = tempfile.TemporaryDirectory()
self.state_dir = self._tmpdir.name
self._env = patch.dict(
os.environ,
{
mcp_session_state.STATE_DIR_ENV: self.state_dir,
mcp_session_state.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
"GITEA_MCP_PROFILE": "prgs-reviewer",
"GITEA_PROFILE_NAME": "prgs-reviewer",
"PYTEST_CURRENT_TEST": "1", # skip runtime stale checks
},
clear=False,
)
self._env.start()
# Mock workspace binding to pass in test context
self._nwb_patch = patch(
"gitea_mcp_server.nwb.assess_namespace_mutation_workspace",
return_value={"block": False, "mutation_workspace": "/tmp/test-worktree"},
)
self._nwb_patch.start()
# Mock authentication headers
self._auth_patch = patch(
"gitea_mcp_server.get_auth_header",
return_value="Bearer mock-token",
)
self._auth_patch.start()
self._username_patch = patch(
"gitea_mcp_server._authenticated_username",
return_value="sysadmin",
)
self._username_patch.start()
# Clear/Mock local session lease in-memory state
reviewer_pr_lease.clear_session_lease()
def tearDown(self):
self._username_patch.stop()
self._auth_patch.stop()
self._nwb_patch.stop()
self._env.stop()
self._tmpdir.cleanup()
reviewer_pr_lease.clear_session_lease()
@patch("gitea_mcp_server.api_request")
def test_save_draft_success(self, mock_api):
print("GITEA_MCP_SERVER FILE:", gitea_mcp_server.__file__)
print("DIR OF GITEA_MCP_SERVER:", dir(gitea_mcp_server))
mock_api.return_value = {
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
validation_commands="pytest tests/",
validation_results="all pass",
blocker_reason="stale daemon",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertTrue(res.get("success"))
self.assertEqual(res.get("head_sha"), "headsha1234567890")
# Load draft and verify fields
draft = mcp_session_state.load_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
self.assertIsNotNone(draft)
self.assertEqual(draft.get("pr_number"), 587)
self.assertEqual(draft.get("action"), "approve")
self.assertEqual(draft.get("body"), "Good changes")
self.assertEqual(draft.get("validation_commands"), "pytest tests/")
self.assertEqual(draft.get("validation_results"), "all pass")
self.assertEqual(draft.get("blocker_reason"), "stale daemon")
self.assertEqual(os.path.realpath(draft.get("worktree_path")), os.path.realpath("/tmp/test-worktree"))
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
@patch("gitea_mcp_server.gitea_submit_pr_review")
def test_resume_draft_and_submit_success(self, mock_submit, mock_comments, mock_api):
# 1. Save draft
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
# Seed local session lease
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "session-1234",
})
# Mock Gitea comments to return active reviewer lease owned by us
mock_comments.return_value = [
{
"id": 1,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: session-1234\n"
"phase: claimed\n"
),
}
]
mock_submit.return_value = {"performed": True, "success": True}
# 2. Resume draft
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
submit=True,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertTrue(res.get("success"))
self.assertTrue(res.get("marked_ready"))
self.assertTrue(res.get("submitted"))
mock_submit.assert_called_once()
# Verify draft was deleted after successful submit
draft = mcp_session_state.load_state(
kind=mcp_session_state.KIND_REVIEW_DRAFT,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
self.assertIsNone(draft)
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._fetch_pr_comments")
def test_resume_draft_fails_checks(self, mock_comments, mock_api):
# Save a draft
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
gitea_mcp_server.gitea_save_review_draft(
pr_number=587,
action="approve",
body="Good changes",
expected_head_sha="headsha1234567890",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
# Seed local session lease
reviewer_pr_lease.record_session_lease({
"pr_number": 587,
"session_id": "session-1234",
})
# Mock Gitea comments to return active reviewer lease owned by us
mock_comments.return_value = [
{
"id": 1,
"user": {"username": "sysadmin"},
"body": (
"<!-- mcp-review-lease:v1 -->\n"
"repo: Scaled-Tech-Consulting/Gitea-Tools\n"
"pr: #587\n"
"reviewer_identity: sysadmin\n"
"profile: prgs-reviewer\n"
"session_id: session-1234\n"
"phase: claimed\n"
),
}
]
# Case A: PR closed
mock_api.return_value = {
"state": "closed",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("PR #587 is not open", res.get("reasons")[0])
# Case B: Head changed
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha_NEW"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("PR head SHA changed", res.get("reasons")[0])
# Case C: Target branch changed
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha_NEW"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/test-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("target branch SHA changed", res.get("reasons")[0])
# Case D: Worktree mismatch
mock_api.return_value = {
"state": "open",
"head": {"sha": "headsha1234567890"},
"base": {"ref": "master", "sha": "basesha0987654321"},
}
res = gitea_mcp_server.gitea_resume_review_draft(
pr_number=587,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="/tmp/different-worktree",
)
self.assertFalse(res.get("success"))
self.assertIn("worktree binding mismatch", res.get("reasons")[0])
+108
View File
@@ -237,5 +237,113 @@ class TestReviewerLeaseMcpGate(unittest.TestCase):
self.assertTrue(any("lease" in r.lower() for r in reasons)) self.assertTrue(any("lease" in r.lower() for r in reasons))
class TestReviewerLeaseHandoffDiagnose(unittest.TestCase):
"""#599: canonical next-action diagnosis for open-PR lease handoff."""
def setUp(self):
leases.clear_session_lease()
def test_no_lease_next_action_acquire(self):
result = leases.diagnose_reviewer_pr_lease_handoff(
[],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592",
)
self.assertEqual(result["classification"], "no_lease")
self.assertEqual(result["next_action"], leases.NEXT_ACTION_ACQUIRE)
self.assertIsNone(result["active_lease"])
def test_foreign_active_wait_pr592_style(self):
# Instructed lease gone; newer foreign claim is active.
old = _lease_comment(592, "23139-da018dab43ba", phase="released")
old["id"] = 8640
active = _lease_comment(592, "51515-2bfb70f0685f", phase="claimed")
active["id"] = 8647
result = leases.diagnose_reviewer_pr_lease_handoff(
[old, active],
pr_number=592,
current_session_id="fresh-session",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592-issue-590",
instructed_session_id="23139-da018dab43ba",
instructed_comment_id=8640,
)
self.assertEqual(
result["classification"],
"instructed_lease_missing_with_replacement",
)
self.assertEqual(result["next_action"], leases.NEXT_ACTION_WAIT)
self.assertTrue(result["instructed_lease_missing_with_replacement"])
self.assertEqual(result["active_lease"]["session_id"], "51515-2bfb70f0685f")
self.assertEqual(result["active_lease"]["comment_id"], 8647)
self.assertFalse(result["mutation_allowed"])
def test_foreign_reclaimable_release_expired(self):
reclaim = _lease_comment(
592, "foreign-old", phase="claimed", minutes_ago=65
)
reclaim["id"] = 99
result = leases.diagnose_reviewer_pr_lease_handoff(
[reclaim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="sysadmin",
proposed_worktree="branches/review-pr-592",
)
self.assertEqual(result["classification"], "foreign_reclaimable")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_RELEASE_EXPIRED_LEASE
)
def test_own_active_resume(self):
head = "a" * 40
claim = _lease_comment(592, "me-1", phase="claimed", candidate_head=head)
claim["id"] = 10
leases.record_session_lease({
"pr_number": 592,
"session_id": "me-1",
"candidate_head": head,
"comment_id": 10,
}, lease_provenance=mla.build_lease_provenance(
source=mla.SOURCE_ACQUIRE,
comment_id=10,
))
result = leases.diagnose_reviewer_pr_lease_handoff(
[claim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="rev1",
proposed_worktree="branches/review-pr382",
)
self.assertEqual(result["classification"], "own_active")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_RESUME_EXACT_OWNER_SESSION
)
self.assertTrue(result["mutation_allowed"])
def test_worktree_binding_mismatch(self):
claim = _lease_comment(592, "me-1", phase="claimed")
claim["id"] = 11
# _lease_comment uses branches/review-pr382
result = leases.diagnose_reviewer_pr_lease_handoff(
[claim],
pr_number=592,
current_session_id="me-1",
current_reviewer_identity="rev1",
proposed_worktree="branches/review-pr-592-issue-590",
env_bound_worktree="branches/review-pr-538",
)
self.assertEqual(result["classification"], "worktree_binding_mismatch")
self.assertEqual(
result["next_action"], leases.NEXT_ACTION_REPAIR_WORKTREE_BINDING
)
self.assertFalse(result["worktree_binding"]["match"])
self.assertFalse(result["mutation_allowed"])
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+41 -1
View File
@@ -52,6 +52,20 @@ CONFIG = {
], ],
"execution_profile": "prgs-reviewer", "execution_profile": "prgs-reviewer",
}, },
"prgs-merger": {
"enabled": True,
"context": "ctx",
"role": "merger",
"username": "sysadmin",
"auth": {"type": "env", "name": "GITEA_TOKEN_MERGER"},
"allowed_operations": [
"gitea.read", "gitea.pr.merge", "gitea.issue.comment",
],
"forbidden_operations": [
"gitea.pr.create", "gitea.branch.push", "gitea.pr.approve",
],
"execution_profile": "prgs-merger",
},
"prgs-reconciler": { "prgs-reconciler": {
"enabled": True, "enabled": True,
"context": "ctx", "context": "ctx",
@@ -104,6 +118,7 @@ class TestRoleSessionRouter(unittest.TestCase):
"GITEA_MCP_PROFILE": profile, "GITEA_MCP_PROFILE": profile,
"GITEA_TOKEN_AUTHOR": "author-pass", "GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TOKEN_REVIEWER": "reviewer-pass", "GITEA_TOKEN_REVIEWER": "reviewer-pass",
"GITEA_TOKEN_MERGER": "merger-pass",
"GITEA_TOKEN_RECONCILER": "reconciler-pass", "GITEA_TOKEN_RECONCILER": "reconciler-pass",
} }
@@ -227,6 +242,31 @@ class TestRoleSessionRouter(unittest.TestCase):
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED) self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
self.assertTrue(route["downstream_allowed"]) self.assertTrue(route["downstream_allowed"])
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
def test_merge_task_under_reviewer_profile_wrong_role_stop(self, _auth, _api):
# #539: a reviewer session must not pass the pre-task merge route
# even if its config accidentally includes gitea.pr.merge.
with patch.dict(os.environ, self._env("prgs-reviewer")):
route = mcp_server.gitea_route_task_session(
task_type="merge_pr", remote="prgs"
)
self.assertEqual(route["required_role"], "merger")
self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE)
self.assertFalse(route["downstream_allowed"])
self.assertIn("merger", route["message"].lower())
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
@patch("mcp_server.get_auth_header", return_value="token merger-pass")
def test_merge_task_under_merger_profile_allowed(self, _auth, _api):
with patch.dict(os.environ, self._env("prgs-merger")):
route = mcp_server.gitea_route_task_session(
task_type="merge_pr", remote="prgs"
)
self.assertEqual(route["required_role"], "merger")
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
self.assertTrue(route["downstream_allowed"])
@patch("mcp_server.api_request", return_value={"login": "sysadmin"}) @patch("mcp_server.api_request", return_value={"login": "sysadmin"})
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass") @patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
def test_author_task_under_reviewer_profile_routes_to_author(self, _auth, _api): def test_author_task_under_reviewer_profile_routes_to_author(self, _auth, _api):
@@ -334,4 +374,4 @@ class TestCheckMidMerge(unittest.TestCase):
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+62
View File
@@ -0,0 +1,62 @@
"""Regression: durable session-state isolation survives env clears (#590)."""
from __future__ import annotations
import os
from pathlib import Path
from unittest.mock import patch
import mcp_session_state
def test_default_state_dir_survives_env_clear():
"""conftest pins default_state_dir so clear=True cannot hit host cache."""
isolated = os.environ.get("GITEA_MCP_SESSION_STATE_DIR")
assert isolated, "conftest must set GITEA_MCP_SESSION_STATE_DIR"
host_cache = Path.home() / ".cache" / "gitea-tools" / "session-state"
with patch.dict(os.environ, {}, clear=True):
assert os.environ.get("GITEA_MCP_SESSION_STATE_DIR") is None
resolved = mcp_session_state.default_state_dir()
assert resolved == isolated
assert Path(resolved).resolve() != host_cache.resolve()
def test_decision_lock_save_load_stays_in_isolated_dir():
"""Review-mutation ledger files land only under the per-test state dir."""
isolated = os.environ.get("GITEA_MCP_SESSION_STATE_DIR")
assert isolated
with patch.dict(os.environ, {}, clear=True):
saved = mcp_session_state.save_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity="gitea-default",
payload={
"live_mutations": [
{"pr_number": 1, "action": "request_changes"}
],
},
)
assert saved is not None
path = mcp_session_state.state_file_path(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity="gitea-default",
)
assert path.startswith(isolated + os.sep) or path.startswith(
isolated + "/"
)
loaded = mcp_session_state.load_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity="gitea-default",
)
assert loaded is not None
assert loaded.get("live_mutations")
host_path = (
Path.home()
/ ".cache"
/ "gitea-tools"
/ "session-state"
/ "review_decision_lock-gitea-default.json"
)
# Host file may exist from operator use; this write must not refresh it.
# Prove our isolated file is distinct and present.
assert Path(path).is_file()
assert Path(path).resolve() != host_path.resolve()
@@ -0,0 +1,378 @@
"""Stale #332 review-decision lock cleanup (#594).
Proves:
a. active terminal locks still block unrelated terminal reviews
b. merged/closed PR locks can be cleared canonically
c. cleanup cannot bypass review gates on open PRs
d. after moot cleanup, mark_ready for a different PR can proceed
(PR #592-style after PR #586-style stale approve)
"""
from __future__ import annotations
import os
import unittest
from unittest.mock import patch
import mcp_server
import stale_review_decision_lock as srdl
from mcp_server import (
gitea_cleanup_stale_review_decision_lock,
gitea_mark_final_review_decision,
)
HEAD = "a" * 40
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
REVIEWER_ENV = {
"GITEA_PROFILE_NAME": "prgs-reviewer",
"GITEA_ALLOWED_OPERATIONS": (
"gitea.read,gitea.pr.review,gitea.pr.approve,"
"gitea.pr.request_changes,gitea.pr.comment"
),
"GITEA_SESSION_PROFILE_LOCK": "prgs-reviewer",
}
REVIEWER_PROFILE = {
"profile_name": "prgs-reviewer",
"allowed_operations": [
"gitea.read",
"gitea.pr.review",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.comment",
],
"forbidden_operations": [
"gitea.pr.merge",
"gitea.pr.create",
"gitea.branch.push",
],
}
def _reviewer_profile_patch():
return patch.object(mcp_server, "get_profile", return_value=REVIEWER_PROFILE)
def _lock(mutations=None, correction=False):
return {
"task": "review_pr",
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"session_pid": os.getpid(),
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"profile_identity": "prgs-reviewer",
"final_review_decision_ready": False,
"ready_pr_number": None,
"ready_action": None,
"ready_expected_head_sha": None,
"ready_remote": None,
"ready_org": None,
"ready_repo": None,
"live_mutations": list(mutations or []),
"correction_authorized": correction,
"correction_reason": None,
}
APPROVED_586 = {
"pr_number": 586,
"action": "approve",
"review_id": 395,
"review_state": "approve",
}
APPROVED_OPEN = {
"pr_number": 700,
"action": "approve",
"review_id": 1,
"review_state": "approve",
}
RC_OPEN = {
"pr_number": 701,
"action": "request_changes",
"review_id": 2,
"review_state": "request_changes",
}
def _seed(mutations=None, correction=False):
import review_workflow_load
review_workflow_load.record_review_workflow_load(mcp_server.PROJECT_ROOT)
mcp_server._save_review_decision_lock(_lock(mutations, correction))
mcp_server.gitea_load_review_workflow()
def _merged_pr(pr_number=586, sha="6913ac9" + "0" * 33):
return {
"number": pr_number,
"state": "closed",
"merged": True,
"merged_at": "2026-07-09T18:20:03Z",
"merge_commit_sha": sha,
}
def _open_pr(pr_number=700):
return {
"number": pr_number,
"state": "open",
"merged": False,
"merged_at": None,
"merge_commit_sha": None,
}
# --------------------------------------------------------------------------- #
# Pure assessment
# --------------------------------------------------------------------------- #
class TestAssessStaleLock(unittest.TestCase):
def test_no_lock(self):
a = srdl.assess_stale_review_decision_lock(None)
self.assertFalse(a["cleanup_allowed"])
self.assertFalse(a["is_moot"])
def test_no_terminal_mutations(self):
a = srdl.assess_stale_review_decision_lock(_lock([]))
self.assertFalse(a["cleanup_allowed"])
def test_open_pr_not_cleanable(self):
a = srdl.assess_stale_review_decision_lock(
_lock([APPROVED_OPEN]), pr_live=_open_pr(700)
)
self.assertFalse(a["is_moot"])
self.assertFalse(a["cleanup_allowed"])
self.assertTrue(any("open PR" in r for r in a["reasons"]))
def test_merged_pr_is_moot_and_cleanable(self):
a = srdl.assess_stale_review_decision_lock(
_lock([APPROVED_586]), pr_live=_merged_pr(586)
)
self.assertTrue(a["is_moot"])
self.assertTrue(a["cleanup_allowed"])
self.assertEqual(a["last_terminal_pr"], 586)
def test_closed_unmerged_is_moot(self):
a = srdl.assess_stale_review_decision_lock(
_lock([RC_OPEN]),
pr_live={"number": 701, "state": "closed", "merged": False},
)
self.assertTrue(a["is_moot"])
self.assertTrue(a["cleanup_allowed"])
def test_profile_mismatch_blocks(self):
a = srdl.assess_stale_review_decision_lock(
_lock([APPROVED_586]),
pr_live=_merged_pr(586),
active_profile_identity="other-profile",
)
self.assertFalse(a["cleanup_allowed"])
self.assertFalse(a["profile_match"])
def test_lookup_error_fail_closed(self):
a = srdl.assess_stale_review_decision_lock(
_lock([APPROVED_586]),
pr_lookup_error="timeout",
)
self.assertFalse(a["cleanup_allowed"])
self.assertTrue(any("timeout" in r for r in a["reasons"]))
# --------------------------------------------------------------------------- #
# Hard-stop still blocks active locks
# --------------------------------------------------------------------------- #
class TestActiveHardStopPreserved(unittest.TestCase):
def tearDown(self):
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def test_open_approve_still_blocks_other_pr_mark_ready(self):
_seed([APPROVED_OPEN])
reasons = mcp_server.terminal_review_hard_stop_reasons(592, "mark_ready")
self.assertTrue(reasons)
self.assertIn("#332", reasons[0])
self.assertIn("gitea_cleanup_stale_review_decision_lock", reasons[0])
def test_request_changes_still_blocks_everything(self):
_seed([RC_OPEN])
for op in ("merge", "mark_ready", "review"):
self.assertTrue(
mcp_server.terminal_review_hard_stop_reasons(592, op),
msg=op,
)
# --------------------------------------------------------------------------- #
# MCP cleanup tool
# --------------------------------------------------------------------------- #
class TestCleanupTool(unittest.TestCase):
def setUp(self):
self._env = patch.dict(os.environ, REVIEWER_ENV, clear=False)
self._env.start()
def tearDown(self):
self._env.stop()
mcp_server._save_review_decision_lock(None)
mcp_server.review_workflow_load.clear_review_workflow_load()
def test_read_only_reports_moot_without_clearing(self):
_seed([APPROVED_586])
with patch.object(mcp_server, "api_request", return_value=_merged_pr()), \
patch.object(mcp_server, "_auth", return_value=FAKE_AUTH), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
), \
_reviewer_profile_patch(), \
patch.object(mcp_server, "_profile_operation_gate", return_value=[]):
r = gitea_cleanup_stale_review_decision_lock(
apply=False, remote="prgs",
org="Scaled-Tech-Consulting", repo="Gitea-Tools",
)
self.assertTrue(r["success"])
self.assertTrue(r["is_moot"])
self.assertTrue(r["cleanup_allowed"])
self.assertFalse(r["cleanup_performed"])
self.assertIsNotNone(mcp_server._load_review_decision_lock())
def test_apply_clears_moot_lock(self):
_seed([APPROVED_586])
posts = []
def _api(method, url, auth, payload=None):
if method == "GET" and "/pulls/586" in url:
return _merged_pr()
if method == "POST" and "/comments" in url:
posts.append(payload)
return {"id": 9999}
return {}
with patch.object(mcp_server, "api_request", side_effect=_api), \
patch.object(mcp_server, "_auth", return_value=FAKE_AUTH), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
), \
_reviewer_profile_patch(), \
patch.object(mcp_server, "_profile_operation_gate", return_value=[]), \
patch.object(
mcp_server, "_audited",
side_effect=lambda *a, **k: __import__(
"contextlib"
).nullcontext(),
):
r = gitea_cleanup_stale_review_decision_lock(
apply=True,
expected_terminal_pr=586,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(r["cleanup_performed"], r)
self.assertTrue(r["audit"]["applied"])
self.assertIsNone(mcp_server._load_review_decision_lock())
self.assertEqual(r.get("audit_comment_id"), 9999)
self.assertTrue(posts)
def test_apply_refuses_open_pr(self):
_seed([APPROVED_OPEN])
with patch.object(mcp_server, "api_request", return_value=_open_pr(700)), \
patch.object(mcp_server, "_auth", return_value=FAKE_AUTH), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
), \
_reviewer_profile_patch(), \
patch.object(mcp_server, "_profile_operation_gate", return_value=[]):
r = gitea_cleanup_stale_review_decision_lock(
apply=True, remote="prgs",
org="Scaled-Tech-Consulting", repo="Gitea-Tools",
)
self.assertFalse(r["cleanup_performed"])
self.assertFalse(r["cleanup_allowed"])
self.assertIsNotNone(mcp_server._load_review_decision_lock())
def test_expected_terminal_pr_pin(self):
_seed([APPROVED_586])
with patch.object(mcp_server, "api_request", return_value=_merged_pr()), \
patch.object(mcp_server, "_auth", return_value=FAKE_AUTH), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
), \
_reviewer_profile_patch(), \
patch.object(mcp_server, "_profile_operation_gate", return_value=[]):
r = gitea_cleanup_stale_review_decision_lock(
apply=True,
expected_terminal_pr=999,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertFalse(r["cleanup_performed"])
self.assertTrue(any("expected_terminal_pr" in x for x in r["reasons"]))
def test_after_moot_cleanup_other_pr_mark_ready_unblocked(self):
"""PR #592-style: after clearing merged #586 lock, new mark_ready works."""
_seed([APPROVED_586])
with patch.object(mcp_server, "api_request", return_value=_merged_pr()), \
patch.object(mcp_server, "_auth", return_value=FAKE_AUTH), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
), \
_reviewer_profile_patch(), \
patch.object(mcp_server, "_profile_operation_gate", return_value=[]), \
patch.object(
mcp_server, "_audited",
side_effect=lambda *a, **k: __import__(
"contextlib"
).nullcontext(),
):
cleaned = gitea_cleanup_stale_review_decision_lock(
apply=True,
expected_terminal_pr=586,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
post_audit_comment=False,
)
self.assertTrue(cleaned["cleanup_performed"], cleaned)
# Hard-stop gone; re-init clean lock like a fresh review_pr resolve.
mcp_server.init_review_decision_lock(remote="prgs", task="review_pr")
mcp_server.gitea_load_review_workflow()
self.assertEqual(
mcp_server.terminal_review_hard_stop_reasons(592, "mark_ready"),
[],
)
no_lease = {"block": False, "reasons": [], "mutation_allowed": True}
with patch.object(mcp_server, "_list_pr_lease_comments", return_value=[]), \
patch.object(
mcp_server, "_pr_work_lease_reviewer_block", return_value=no_lease
), \
patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={
"eligible": True,
"reasons": [],
"authenticated_user": "sysadmin",
"pr_author": "jcwalker3",
"self_author": False,
},
), \
patch.object(
mcp_server, "_authenticated_username", return_value="sysadmin"
):
marked = gitea_mark_final_review_decision(
pr_number=592,
action="approve",
expected_head_sha=HEAD,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(marked.get("marked_ready"), marked)
if __name__ == "__main__":
unittest.main()