Compare commits

..
Author SHA1 Message Date
sysadmin a0fffae576 docs: ADR for stable MCP control runtime vs dev runtime
Policy: real Gitea mutations use stable control runtime only; normal
sessions must not kill/restart/relaunch from worktrees or edit the stable
checkout. Promotion is operator-only with proof and rollback.

Refs: #615
2026-07-09 20:16:50 -04:00
13 changed files with 205 additions and 4340 deletions
-610
View File
@@ -1,610 +0,0 @@
"""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
@@ -1,63 +0,0 @@
# 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
```
@@ -1,301 +0,0 @@
# 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 |
@@ -0,0 +1,156 @@
# ADR: Stable control runtime vs dev runtime (Gitea MCP)
- **Status:** Accepted (policy effective immediately for LLM sessions; tooling may lag)
- **Date:** 2026-07-09
- **Tracking issue:** [#615](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/615)
- **Related:**
- [#543](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/543) / `docs/mcp-namespace-health.md` — client-namespace health
- `docs/mcp-namespace-eof-recovery.md` — reconnect-only EOF recovery (no PID kill)
- [#558](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/558) / `docs/mcp-daemon-import-guard.md` — sanctioned daemon
- [#557](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/557) / `docs/bootstrap-review-path.md` — controller bootstrap for self-hosted fixes
- Allocator / control-plane ADR: `docs/architecture/mcp-allocator-control-plane-observability-adr.md` (#613 / PR #614)
## 1. Context
The Gitea MCP server is the **control plane** for real issue/PR mutations (create, comment, lock, review, merge, etc.). When author/reviewer/merger/reconciler sessions kill or restart that process, relaunch it from a feature worktree, or edit the checkout that process loads, operators observe:
- Mid-session identity/preflight resets
- Stale-runtime vs master parity failures
- IDE transport EOF / “tool not found” while code on disk has changed
- Accidental production mutations from experimental code
This ADR separates **stable control runtime** from **dev/test runtime** and defines promotion proof.
## 2. Decision
### 2.1 Stable control runtime
The Gitea MCP server used for **real workflow mutations** is the **stable control runtime**.
Characteristics:
- Loads a known, promoted revision of Gitea-Tools (or the packaged release layout operators designate)
- Registered in the IDE/client as the production namespaces (`gitea-tools`, `gitea-reviewer`, `gitea-merger`, `gitea-reconciler`, etc.)
- Holds production profile credentials via sanctioned keychain/env paths only
### 2.2 Dev / test runtime
MCP **server code** development and testing:
- Happens in isolated **`branches/`** worktrees (or other non-stable checkouts)
- May use a **separate** dev/test MCP runtime/process when process-level testing is required
- **Must not** be used for real Gitea mutations on production issues/PRs
### 2.3 Forbidden actions (normal sessions)
Normal **author, reviewer, merger, and reconciler** sessions **must not**:
| Forbidden | Why |
|-----------|-----|
| Kill the running MCP server process | Drops all concurrent sessions; loses preflight state |
| Restart the MCP server process | Same as kill; causes stale/identity churn mid-workflow |
| Relaunch MCP from a development worktree | Runs unpromoted code against production mutations |
| Edit files in the stable runtime checkout | Hot-mutates control plane under concurrent users |
| Use experimental/dev MCP for real Gitea mutations | Bypasses promotion proof and audit expectations |
EOF / transport recovery: **client reconnect only** (see `docs/mcp-namespace-eof-recovery.md`). Do not “fix” health by killing PIDs or bumping MCP config mtimes as a normal session procedure.
### 2.4 Promotion (operator / release-manager only)
Promotion of a new revision into the stable control runtime is an **explicit operator/release-manager action**, not an LLM self-service step.
A promotion **must record** (issue comment, release note, or promotion ledger):
| Field | Description |
|-------|-------------|
| **previous runtime SHA** | Commit previously loaded by stable runtime |
| **promoted runtime SHA** | Commit after promotion |
| **source branch/PR** | Where the change was reviewed |
| **restart/reload method** | How the process was cycled (e.g. supervised restart, client reload) |
| **health check proof** | Client-namespace probe success (`gitea_whoami` / namespace health) |
| **identity/profile proof** | Expected profile(s) and username(s) after reload |
| **workspace/root proof** | Stable checkout path / root matches intended layout |
| **mutation capability proof** | Required permissions for the target role present; forbidden ops still forbidden |
| **rollback instructions** | How to restore previous SHA and re-verify health |
Suggested durable marker:
```text
## MCP STABLE RUNTIME PROMOTION (#615)
Status: COMPLETED | ROLLED_BACK | ABORTED
Previous-SHA: <full sha>
Promoted-SHA: <full sha>
Source-PR: <number>
Source-Branch: <name>
Reload-Method: <text>
Health-Proof: client_namespace whoami OK / assess_mcp_namespace_health OK
Identity-Proof: profile=<name> user=<name>
Workspace-Proof: root=<path>
Mutation-Proof: allowed_ops include <…>; forbidden include <…>
Rollback: checkout <previous sha>; reload method <…>; re-run health/identity proofs
Operator: <username>
Timestamp: <ISO-8601>
```
### 2.5 Unhealthy stable runtime → stop work
If the stable MCP runtime is **unhealthy** (client-namespace probes fail, wrong identity, wrong root, missing mutation capability, persistent EOF after reconnect):
1. **Normal PR / review / merge / issue-mutation work must stop.**
2. Do **not** improvise by switching to a dev worktree MCP for production mutations.
3. Resume only after:
- runtime is restored, **or**
- a **controlled** promotion/rollback completes with the promotion record above, **or**
- a controller invokes the narrow **bootstrap review path** (#557) when the defect is self-hosted and documented.
## 3. Relationship to other controls
| Doc / mechanism | Interaction |
|-----------------|-------------|
| Namespace health (#543) | Proves IDE client can call tools; does not authorize restart |
| EOF recovery | Reconnect only; no process kill |
| Daemon import guard (#558) | Mutations require sanctioned daemon; not a bare shell import |
| Bootstrap path (#557) | Only controller-authorized exception when live runtime cannot review its own fix |
| Allocator / control-plane ADR | Coordination DB is separate; still depends on a healthy MCP surface for Gitea writes |
## 4. Consequences
### Positive
- Predictable control plane for concurrent LLMs
- Clear operator-only promotion gate with rollback
- Aligns session behavior with health/EOF docs already landed
### Costs
- LLM sessions must wait when runtime is sick (no DIY restart)
- Operators must maintain promotion discipline and dual-runtime config if they use a dev MCP
### Non-goals
- Does not ban operator-supervised restarts during incidents
- Does not replace CI or code review for MCP changes
- Does not authorize editing stable checkout “because tests need a quick fix”
## 5. Implementation follow-ups (optional tooling)
These may land in later issues; the **policy binds sessions now**:
1. Session preflight that refuses mutations if workspace root equals a `branches/` feature worktree configured as “dev only.”
2. Explicit `runtime_kind=stable|dev` in MCP config and `gitea_whoami` profile metadata.
3. Promotion checklist script that emits the durable promotion marker fields.
4. Operator Guide wiki cross-link to this ADR.
## 6. Acceptance for this ADR
1. Document merged under `docs/architecture/`.
2. Issue #615 references this path.
3. LLM/operator runbooks treat kill/restart/relaunch-from-worktree as **violations**.
4. Unhealthy runtime stops normal mutation work until restore/promotion/rollback/bootstrap.
## 7. Document history
| Date | Change |
|------|--------|
| 2026-07-09 | Initial ADR: stable vs dev runtime, forbidden session actions, promotion proof fields, stop-work rule |
+42 -420
View File
@@ -21,7 +21,6 @@ import json
import functools import functools
import contextlib import contextlib
import subprocess import subprocess
import uuid
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
@@ -833,8 +832,6 @@ import review_workflow_boundary # noqa: E402
import review_workflow_load # noqa: E402 import review_workflow_load # noqa: E402
import mcp_session_state # noqa: E402 import mcp_session_state # noqa: E402
import stale_review_decision_lock # noqa: E402 import stale_review_decision_lock # noqa: E402
import allocator_service # noqa: E402
import control_plane_db # noqa: E402
import agent_temp_artifacts import agent_temp_artifacts
import issue_lock_worktree # noqa: E402 import issue_lock_worktree # noqa: E402
import issue_lock_provenance # noqa: E402 import issue_lock_provenance # noqa: E402
@@ -1370,26 +1367,8 @@ def cleanup_in_progress_for_pr(pr_payload: dict, remote: str, host: str | None,
# ── Helpers ─────────────────────────────────────────────────────────────────── # ── Helpers ───────────────────────────────────────────────────────────────────
def _effective_remote(remote: str) -> str:
"""If remote is the default ('dadeschools') but the active profile base_url maps to a known remote, use that remote instead."""
try:
profile = get_profile()
base_url = profile.get("base_url")
if remote == "dadeschools" and base_url:
import urllib.parse
url = urllib.parse.urlparse(base_url)
host = (url.netloc or url.path or "").strip().lower()
for k, v in REMOTES.items():
if v.get("host") == host:
return k
except Exception:
pass
return remote
def _resolve(remote: str, host: str | None, org: str | None, repo: str | None): def _resolve(remote: str, host: str | None, org: str | None, repo: str | None):
"""Resolve remote + overrides to (host, org, repo).""" """Resolve remote + overrides to (host, org, repo)."""
remote = _effective_remote(remote)
if remote not in REMOTES: if remote not in REMOTES:
raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}") raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}")
profile = REMOTES[remote] profile = REMOTES[remote]
@@ -1423,7 +1402,6 @@ def _resolve_control_plane_guide_target(
if org is not None and repo is not None: if org is not None and repo is not None:
return _resolve(remote, host, org, repo) return _resolve(remote, host, org, repo)
remote = _effective_remote(remote)
if remote not in REMOTES: if remote not in REMOTES:
raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}") raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}")
@@ -3098,26 +3076,22 @@ def check_review_decision_gate(
) )
prior = list(lock.get("live_mutations") or []) prior = list(lock.get("live_mutations") or [])
ready_head = lock.get("ready_expected_head_sha") if prior and not lock.get("correction_authorized"):
if stale_review_decision_lock.prior_live_mutations_block_boundary( reasons.append(
lock, pr_number=pr_number, expected_head_sha=ready_head "live review mutation already recorded in this run; only one live "
"review mutation is allowed unless "
"gitea_authorize_review_correction was invoked (fail closed)"
)
elif (
action in _TERMINAL_REVIEW_ACTIONS
and any(m.get("action") in _TERMINAL_REVIEW_ACTIONS for m in prior)
and not lock.get("correction_authorized")
): ):
if prior and not lock.get("correction_authorized"): reasons.append(
reasons.append( "terminal review decision already submitted on this PR in this "
"live review mutation already recorded for this PR head in this " "run; blocked unless an operator-approved correction was "
"run; only one live review mutation is allowed per head unless " "authorized (fail closed)"
"gitea_authorize_review_correction was invoked (fail closed, #620)" )
)
elif (
action in _TERMINAL_REVIEW_ACTIONS
and any(m.get("action") in _TERMINAL_REVIEW_ACTIONS for m in prior)
and not lock.get("correction_authorized")
):
reasons.append(
"terminal review decision already submitted for this PR head in "
"this run; blocked unless an operator-approved correction was "
"authorized (fail closed, #620)"
)
return reasons return reasons
@@ -3125,18 +3099,12 @@ def check_review_decision_gate(
def record_live_review_mutation(pr_number: int, action: str, review_id: int | None = None): def record_live_review_mutation(pr_number: int, action: str, review_id: int | None = None):
lock = _load_review_decision_lock() or {} lock = _load_review_decision_lock() or {}
mutations = list(lock.get("live_mutations") or []) mutations = list(lock.get("live_mutations") or [])
head_sha = stale_review_decision_lock.normalize_head_sha( mutations.append({
lock.get("ready_expected_head_sha")
)
entry = {
"pr_number": pr_number, "pr_number": pr_number,
"action": action, "action": action,
"review_id": review_id, "review_id": review_id,
"review_state": action, "review_state": action,
} })
if head_sha:
entry["head_sha"] = head_sha
mutations.append(entry)
lock["live_mutations"] = mutations lock["live_mutations"] = mutations
if lock.get("correction_authorized"): if lock.get("correction_authorized"):
lock["correction_authorized"] = False lock["correction_authorized"] = False
@@ -3144,28 +3112,17 @@ def record_live_review_mutation(pr_number: int, action: str, review_id: int | No
_save_review_decision_lock(lock) _save_review_decision_lock(lock)
def terminal_review_hard_stop_reasons( def terminal_review_hard_stop_reasons(pr_number: int, operation: str) -> list[str]:
pr_number: int, """Session hard-stop after a terminal live review mutation (#332).
operation: str,
expected_head_sha: str | None = None,
) -> list[str]:
"""Session hard-stop after a terminal live review mutation (#332 / #620).
After a terminal verdict is consumed for a given PR **head**, the only After a terminal verdict is consumed in this run, the only permitted
permitted continuation is the merge sequence for that same approved PR continuation is the merge sequence for the same PR that was approved.
(merge does not require a new head). A REQUEST_CHANGES (or an approval of A REQUEST_CHANGES (or an approval of a different PR) blocks every
a different PR, or the same head) blocks further review/mark-ready/merge further review/mark-ready/merge mutation. An operator-approved
mutations for that boundary. correction (#211) re-opens the review path only — never a cross-PR
merge.
#620: when *expected_head_sha* differs from the last terminal's reviewed *operation* is one of 'merge', 'mark_ready', or 'review'.
head on the **same open PR**, mark_ready / review / resume may proceed so
a fresh formal decision can be recorded for the new head. Historical
mutations are preserved.
An operator-approved correction (#211) re-opens the review path only —
never a cross-PR merge.
*operation* is one of 'merge', 'mark_ready', 'review', or 'resume'.
Returns [] when the operation may proceed. Returns [] when the operation may proceed.
""" """
lock = _load_review_decision_lock() lock = _load_review_decision_lock()
@@ -3184,19 +3141,8 @@ def terminal_review_hard_stop_reasons(
and last.get("pr_number") == pr_number and last.get("pr_number") == pr_number
): ):
return [] return []
if operation in ("mark_ready", "review", "resume") and lock.get( if operation in ("mark_ready", "review") and lock.get("correction_authorized"):
"correction_authorized"
):
return [] return []
# #620: same PR, different reviewed head → fresh decision boundary.
if stale_review_decision_lock.terminal_boundary_allows_fresh_decision(
lock,
pr_number=pr_number,
expected_head_sha=expected_head_sha,
operation=operation,
):
return []
locked_head = stale_review_decision_lock.mutation_head_sha(last, lock)
if last.get("action") == "approve": if last.get("action") == "approve":
guidance = ( guidance = (
f"only the merge sequence for approved PR " f"only the merge sequence for approved PR "
@@ -3204,22 +3150,15 @@ def terminal_review_hard_stop_reasons(
) )
else: else:
guidance = "the session must stop and produce a final report" guidance = "the session must stop and produce a final report"
head_note = (
f" at head {locked_head[:12]}"
if locked_head
else ""
)
recovery = ( recovery = (
"if that PR is already merged/closed, use " "if that PR is already merged/closed, use "
"gitea_cleanup_stale_review_decision_lock (apply=true) after live-state " "gitea_cleanup_stale_review_decision_lock (apply=true) after live-state "
"proof (#594); if the PR is still open but the head moved, mark/submit " "proof (#594); never delete session-state files by hand"
"with the new expected_head_sha (#620); never delete session-state "
"files by hand"
) )
return [ return [
"terminal review mutation already consumed in this run " "terminal review mutation already consumed in this run "
f"({last.get('action')} on PR #{last.get('pr_number')}{head_note}); " f"({last.get('action')} on PR #{last.get('pr_number')}); {guidance} "
f"{guidance} (fail closed, #332); {recovery}" f"(fail closed, #332); {recovery}"
] ]
@@ -3842,11 +3781,18 @@ def gitea_mark_final_review_decision(
"reasons": workflow_blockers + ( "reasons": workflow_blockers + (
review_workflow_load.recovery_handoff_without_replay()), review_workflow_load.recovery_handoff_without_replay()),
} }
hard_stop = terminal_review_hard_stop_reasons( hard_stop = terminal_review_hard_stop_reasons(pr_number, "mark_ready")
pr_number, "mark_ready", expected_head_sha=expected_head_sha
)
if hard_stop: if hard_stop:
return {"marked_ready": False, "reasons": hard_stop} return {"marked_ready": False, "reasons": hard_stop}
if lock.get("live_mutations") and not lock.get("correction_authorized"):
return {
"marked_ready": False,
"reasons": [
"cannot mark final decision after a live review mutation was "
"already recorded in this run unless an operator-approved "
"correction was authorized"
],
}
if action not in _REVIEW_ACTIONS: if action not in _REVIEW_ACTIONS:
return { return {
"marked_ready": False, "marked_ready": False,
@@ -3863,17 +3809,6 @@ def gitea_mark_final_review_decision(
"decision (fail closed, #399)" "decision (fail closed, #399)"
], ],
} }
if stale_review_decision_lock.prior_live_mutations_block_boundary(
lock, pr_number=pr_number, expected_head_sha=expected_head_sha
):
return {
"marked_ready": False,
"reasons": [
"cannot mark final decision after a live review mutation was "
"already recorded for this PR head unless an operator-approved "
"correction was authorized (fail closed, #620)"
],
}
elig = gitea_check_pr_eligibility( elig = gitea_check_pr_eligibility(
pr_number=pr_number, pr_number=pr_number,
action="review", action="review",
@@ -3926,8 +3861,6 @@ def gitea_mark_final_review_decision(
"#332)" "#332)"
], ],
} }
# Preserve historical terminal head boundaries before advancing ready_* (#620).
stale_review_decision_lock.backfill_terminal_heads_from_ready(lock)
lock["final_review_decision_ready"] = True lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = pr_number lock["ready_pr_number"] = pr_number
lock["ready_action"] = action lock["ready_action"] = action
@@ -3945,7 +3878,6 @@ def gitea_mark_final_review_decision(
"org": org, "org": org,
"repo": repo, "repo": repo,
"final_review_decision_ready": True, "final_review_decision_ready": True,
"head_scoped": True,
"reasons": [], "reasons": [],
} }
@@ -4112,12 +4044,6 @@ def gitea_cleanup_stale_review_decision_lock(
"cleanup_allowed": assessment.get("cleanup_allowed"), "cleanup_allowed": assessment.get("cleanup_allowed"),
"last_terminal_pr": assessment.get("last_terminal_pr"), "last_terminal_pr": assessment.get("last_terminal_pr"),
"last_terminal_action": assessment.get("last_terminal_action"), "last_terminal_action": assessment.get("last_terminal_action"),
"locked_head_sha": assessment.get("locked_head_sha"),
"current_pr_head_sha": assessment.get("current_pr_head_sha"),
"stale_by_head": assessment.get("stale_by_head"),
"fresh_review_on_current_head_allowed": assessment.get(
"fresh_review_on_current_head_allowed"
),
"pr_state": assessment.get("pr_state"), "pr_state": assessment.get("pr_state"),
"pr_merged": assessment.get("pr_merged"), "pr_merged": assessment.get("pr_merged"),
"pr_merged_or_closed": assessment.get("pr_merged_or_closed"), "pr_merged_or_closed": assessment.get("pr_merged_or_closed"),
@@ -8824,7 +8750,6 @@ def gitea_whoami(
metadata: profile_name + allowed_operations; never the token). metadata: profile_name + allowed_operations; never the token).
""" """
record_preflight_check("whoami") record_preflight_check("whoami")
remote = _effective_remote(remote)
if remote not in REMOTES: if remote not in REMOTES:
raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}") raise ValueError(f"Unknown remote '{remote}'. Choose from: {list(REMOTES)}")
h = host or REMOTES[remote]["host"] h = host or REMOTES[remote]["host"]
@@ -8933,6 +8858,8 @@ def gitea_get_profile(
"execution_profile": profile.get("execution_profile"), "execution_profile": profile.get("execution_profile"),
"auth_source_type": profile.get("auth_source_type"), "auth_source_type": profile.get("auth_source_type"),
# Auth is reported as a status only (#120): the token source *name* # Auth is reported as a status only (#120): the token source *name*
# (env var name / keychain id) joins endpoint URLs behind the
# GITEA_MCP_REVEAL_ENDPOINTS admin opt-in. Token values never appear.
"auth_status": ("configured" if profile["token_source_name"] "auth_status": ("configured" if profile["token_source_name"]
else "unconfigured"), else "unconfigured"),
"remote": remote if remote in REMOTES else None, "remote": remote if remote in REMOTES else None,
@@ -8944,7 +8871,6 @@ def gitea_get_profile(
result["base_url"] = profile["base_url"] result["base_url"] = profile["base_url"]
result["server"] = None result["server"] = None
remote = _effective_remote(remote)
if remote not in REMOTES: if remote not in REMOTES:
# Mark ambiguity rather than raising: the tool stays inspectable. # Mark ambiguity rather than raising: the tool stays inspectable.
result["identity_status"] = "unknown" result["identity_status"] = "unknown"
@@ -11112,310 +11038,6 @@ def gitea_capability_stop_terminal_report() -> dict:
}) })
def _control_plane_db_or_error() -> tuple[Any | None, list[str]]:
"""Open the #613 control-plane DB substrate; fail closed on errors."""
try:
db = control_plane_db.ControlPlaneDB()
return db, []
except Exception as exc: # noqa: BLE001
return None, [
f"control-plane DB substrate unavailable: {_redact(str(exc))} "
"(fail closed, #613/#600)"
]
def _allocator_candidates_from_gitea(
*,
remote: str,
host: str | None,
org: str,
repo: str,
include_issues: bool = True,
include_prs: bool = True,
limit: int = 50,
) -> tuple[list[Any], list[str]]:
"""Build allocator candidates from live Gitea open issues/PRs."""
reasons: list[str] = []
candidates: list[Any] = []
try:
h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h)
except Exception as exc: # noqa: BLE001
return [], [f"failed to resolve Gitea target: {_redact(str(exc))}"]
if include_prs:
try:
prs = api_get_all(
f"{repo_api_url(h, o, r)}/pulls?state=open", auth
) or []
except Exception as exc: # noqa: BLE001
reasons.append(f"failed to list open PRs: {_redact(str(exc))}")
prs = []
for pr in prs[: max(1, int(limit))]:
if not isinstance(pr, dict):
continue
number = pr.get("number")
if number is None:
continue
head = pr.get("head") if isinstance(pr.get("head"), dict) else {}
head_sha = head.get("sha") or pr.get("head_sha")
labels = []
for lab in pr.get("labels") or []:
if isinstance(lab, dict) and lab.get("name"):
labels.append(str(lab["name"]))
elif isinstance(lab, str):
labels.append(lab)
# Lightweight review signals (best-effort; fail soft into reviewer path).
rc_current = False
approval_current = False
approval_stale = False
try:
feedback = gitea_get_pr_review_feedback(
int(number), remote=remote, org=o, repo=r
)
if feedback.get("success"):
rc_current = bool(
feedback.get("has_blocking_change_requests")
and not feedback.get("review_feedback_stale")
)
# Stale RC means author pushed; reviewer still next.
if feedback.get("has_blocking_change_requests") and feedback.get(
"review_feedback_stale"
):
approval_stale = False
except Exception:
pass
mergeable = bool(pr.get("mergeable"))
try:
candidates.append(
allocator_service.WorkCandidate(
kind="pr",
number=int(number),
state="open",
labels=tuple(labels),
title=str(pr.get("title") or ""),
priority=10 if rc_current else 5,
head_sha=head_sha,
request_changes_current_head=rc_current,
approval_on_current_head=approval_current,
approval_stale=approval_stale,
mergeable=mergeable,
)
)
except Exception as exc: # noqa: BLE001
reasons.append(
f"skipped invalid PR candidate #{number}: {_redact(str(exc))}"
)
if include_issues:
try:
issues = api_get_all(
f"{repo_api_url(h, o, r)}/issues?state=open&type=issues", auth
) or []
except Exception as exc: # noqa: BLE001
# Fallback without type filter
try:
issues = api_get_all(
f"{repo_api_url(h, o, r)}/issues?state=open", auth
) or []
except Exception as exc2: # noqa: BLE001
reasons.append(
f"failed to list open issues: {_redact(str(exc2))}"
)
issues = []
for issue in issues[: max(1, int(limit))]:
if not isinstance(issue, dict):
continue
# Pull requests also appear in /issues on Gitea — skip them.
if issue.get("pull_request") is not None:
continue
number = issue.get("number")
if number is None:
continue
labels = []
for lab in issue.get("labels") or []:
if isinstance(lab, dict) and lab.get("name"):
labels.append(str(lab["name"]).lower())
elif isinstance(lab, str):
labels.append(lab.lower())
body = str(issue.get("body") or "")
title = str(issue.get("title") or "")
blocked = "status:blocked" in labels
# Explicit downstream dependency: #612 waits on #600 allocator.
dep_unmet = False
dep_reason = None
if int(number) == 612:
dep_unmet = True
dep_reason = (
"issue#612 (incident bridge) remains downstream of #600 "
"allocator and must not be allocated until #600 is complete"
)
# Generic body markers for blocked-on unfinished deps.
lower_body = body.lower()
if "blocked on #600" in lower_body or "downstream of #600" in lower_body:
dep_unmet = True
dep_reason = dep_reason or (
f"issue#{number} body marks dependency on #600"
)
try:
candidates.append(
allocator_service.WorkCandidate(
kind="issue",
number=int(number),
state="open",
labels=tuple(labels),
title=title,
priority=20 if "status:ready" in labels else 1,
blocked=blocked,
dependency_unmet=dep_unmet,
dependency_reason=dep_reason,
)
)
except Exception as exc: # noqa: BLE001
reasons.append(
f"skipped invalid issue candidate #{number}: {_redact(str(exc))}"
)
return candidates, reasons
@mcp.tool()
def gitea_allocate_next_work(
apply: bool = False,
role: str | None = None,
session_id: str | None = None,
remote: str = "dadeschools",
host: str | None = None,
org: str | None = None,
repo: str | None = None,
include_issues: bool = True,
include_prs: bool = True,
candidates_json: str | None = None,
limit: int = 50,
) -> dict:
"""Controller-owned next-work allocator using the #613 control-plane DB (#600).
Workers must not self-select exclusive work under the standard multi-LLM
workflow. Call this tool instead.
*apply=false* (default): dry-run selection only no assignment/lease.
*apply=true*: atomically assign + lease the selected Gitea issue/PR via
``ControlPlaneDB.assign_and_lease`` (never file locks or comment-only
leases as the coordination source).
Outcomes include: ``assigned_work``, ``preview``, ``wait``,
``blocked_by_terminal_path``, ``no_safe_work``, ``role_ineligible``.
*candidates_json* may inject a JSON list of candidate dicts (tests /
controller overrides). When omitted, open issues/PRs are loaded from Gitea.
#612 remains downstream: raw monitoring incidents are never candidates.
"""
read_block = _profile_operation_gate("gitea.read")
if read_block:
return {
"success": False,
"outcome": allocator_service.OUTCOME_NO_SAFE,
"apply": bool(apply),
"reasons": read_block,
"permission_report": _permission_block_report("gitea.read"),
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
try:
h, o, r = _resolve(remote, host, org, repo)
except ValueError as exc:
return {
"success": False,
"outcome": allocator_service.OUTCOME_NO_SAFE,
"reasons": [str(exc)],
"assignment": None,
"substrate": "control_plane_db",
}
profile = get_profile()
profile_name = (profile.get("profile_name") or "").strip() or None
active_role = _profile_role_kind(profile)
role_in = (role or active_role or "").strip() or "author"
try:
username = _authenticated_username(h)
except Exception:
username = None
db, db_errs = _control_plane_db_or_error()
if db is None:
return {
"success": False,
"outcome": allocator_service.OUTCOME_NO_SAFE,
"apply": bool(apply),
"reasons": db_errs,
"assignment": None,
"substrate": "control_plane_db",
"file_lock_only": False,
"comment_lease_only": False,
}
inv_reasons: list[str] = []
candidates: list[Any] = []
if candidates_json:
try:
raw = json.loads(candidates_json)
if not isinstance(raw, list):
raise ValueError("candidates_json must be a JSON list")
for item in raw:
if not isinstance(item, dict):
continue
candidates.append(allocator_service.candidate_from_dict(item))
except Exception as exc: # noqa: BLE001
return {
"success": False,
"outcome": allocator_service.OUTCOME_NO_SAFE,
"apply": bool(apply),
"reasons": [
f"invalid candidates_json: {_redact(str(exc))} (fail closed)"
],
"assignment": None,
"substrate": "control_plane_db",
}
else:
candidates, inv_reasons = _allocator_candidates_from_gitea(
remote=remote,
host=host,
org=o,
repo=r,
include_issues=include_issues,
include_prs=include_prs,
limit=limit,
)
sid = (session_id or "").strip() or (
f"{profile_name or 'session'}-{os.getpid()}-{uuid.uuid4().hex[:8]}"
)
result = allocator_service.allocate_next_work(
db,
session_id=sid,
role=role_in,
remote=remote if remote in REMOTES else remote,
org=o,
repo=r,
candidates=candidates,
apply=bool(apply),
profile_name=profile_name,
username=username,
)
if inv_reasons:
result.setdefault("inventory_warnings", inv_reasons)
result["candidate_count"] = len(candidates)
result["inventory_source"] = (
"candidates_json" if candidates_json else "gitea_live"
)
return result
# ── Entry point ─────────────────────────────────────────────────────────────── # ── Entry point ───────────────────────────────────────────────────────────────
if __name__ == "__main__": if __name__ == "__main__":
-3
View File
@@ -6,9 +6,6 @@ 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,
+6 -194
View File
@@ -1,4 +1,4 @@
"""Stale / moot #332 review-decision lock detection and cleanup policy (#594/#620). """Stale / moot #332 review-decision lock detection and cleanup policy (#594).
#332 correctly hard-stops a reviewer session after a terminal live review #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 mutation. After #559 those locks are durable on disk, so they can outlive the
@@ -6,12 +6,6 @@ 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 same-PR merge sequence remains, yet the durable ledger still blocks unrelated
new terminal reviews. 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 This module is pure policy (no Gitea I/O). The MCP tool
``gitea_cleanup_stale_review_decision_lock`` fetches live PR state, calls these ``gitea_cleanup_stale_review_decision_lock`` fetches live PR state, calls these
helpers, and only then clears durable state when cleanup is allowed. helpers, and only then clears durable state when cleanup is allowed.
@@ -29,45 +23,6 @@ from typing import Any
TERMINAL_REVIEW_ACTIONS = frozenset({"approve", "request_changes"}) 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: def last_terminal_mutation(lock: dict | None) -> dict | None:
"""Return the last terminal live mutation on *lock*, or None.""" """Return the last terminal live mutation on *lock*, or None."""
if not lock: if not lock:
@@ -80,125 +35,18 @@ def last_terminal_mutation(lock: dict | None) -> dict | None:
return terminals[-1] if terminals else None 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]: def classify_pr_live_state(pr_live: dict | None) -> dict[str, Any]:
"""Normalize a Gitea PR payload into merge/closed flags.""" """Normalize a Gitea PR payload into merge/closed flags."""
pr = pr_live or {} pr = pr_live or {}
merged = bool(pr.get("merged") or pr.get("merged_at")) merged = bool(pr.get("merged") or pr.get("merged_at"))
state = (pr.get("state") or "").strip().lower() or None state = (pr.get("state") or "").strip().lower() or None
closed = state == "closed" 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 { return {
"pr_state": state, "pr_state": state,
"pr_merged": merged, "pr_merged": merged,
"pr_merged_or_closed": merged or closed, "pr_merged_or_closed": merged or closed,
"merge_commit_sha": pr.get("merge_commit_sha") "merge_commit_sha": pr.get("merge_commit_sha")
or pr.get("merged_commit_sha"), or pr.get("merged_commit_sha"),
"current_pr_head_sha": head_sha,
} }
@@ -208,7 +56,6 @@ def lock_summary(lock: dict | None) -> dict[str, Any] | None:
return None return None
last = last_terminal_mutation(lock) last = last_terminal_mutation(lock)
mutations = list(lock.get("live_mutations") or []) mutations = list(lock.get("live_mutations") or [])
last_head = mutation_head_sha(last, lock) if last else None
return { return {
"task": lock.get("task"), "task": lock.get("task"),
"remote": lock.get("remote"), "remote": lock.get("remote"),
@@ -220,21 +67,16 @@ def lock_summary(lock: dict | None) -> dict[str, Any] | None:
"session_profile": lock.get("session_profile"), "session_profile": lock.get("session_profile"),
"ready_pr_number": lock.get("ready_pr_number"), "ready_pr_number": lock.get("ready_pr_number"),
"ready_action": lock.get("ready_action"), "ready_action": lock.get("ready_action"),
"ready_expected_head_sha": normalize_head_sha(
lock.get("ready_expected_head_sha")
),
"live_mutations_count": len(mutations), "live_mutations_count": len(mutations),
"last_terminal": ( "last_terminal": (
{ {
"pr_number": last.get("pr_number"), "pr_number": last.get("pr_number"),
"action": last.get("action"), "action": last.get("action"),
"review_id": last.get("review_id"), "review_id": last.get("review_id"),
"head_sha": last_head,
} }
if last if last
else None else None
), ),
"locked_head_sha": last_head,
"correction_authorized": bool(lock.get("correction_authorized")), "correction_authorized": bool(lock.get("correction_authorized")),
"updated_at": lock.get("updated_at") or lock.get("recorded_at"), "updated_at": lock.get("updated_at") or lock.get("recorded_at"),
} }
@@ -246,16 +88,13 @@ def assess_stale_review_decision_lock(
pr_live: dict | None = None, pr_live: dict | None = None,
pr_lookup_error: str | None = None, pr_lookup_error: str | None = None,
active_profile_identity: str | None = None, active_profile_identity: str | None = None,
current_pr_head_sha: str | None = None,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""Assess whether a durable review-decision lock is stale/moot (#594/#620). """Assess whether a durable review-decision lock is stale/moot (#594).
*pr_live* must be the live Gitea payload for the **last terminal *pr_live* must be the live Gitea payload for the **last terminal
mutation's PR**. When no lock / no terminal mutation exists, cleanup is 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 not applicable. When the PR is still open (or lookup fails), cleanup is
forbidden (#594). Open PR + different live head reports forbidden and #332 hard-stop remains in force.
``stale_by_head`` / ``fresh_review_on_current_head_allowed`` (#620)
without enabling cleanup.
""" """
summary = lock_summary(lock) summary = lock_summary(lock)
result: dict[str, Any] = { result: dict[str, Any] = {
@@ -263,10 +102,6 @@ def assess_stale_review_decision_lock(
"lock_summary": summary, "lock_summary": summary,
"last_terminal_pr": None, "last_terminal_pr": None,
"last_terminal_action": 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, "is_moot": False,
"cleanup_allowed": False, "cleanup_allowed": False,
"profile_match": True, "profile_match": True,
@@ -306,10 +141,8 @@ def assess_stale_review_decision_lock(
pr_number = last.get("pr_number") pr_number = last.get("pr_number")
action = last.get("action") action = last.get("action")
locked_head = mutation_head_sha(last, lock)
result["last_terminal_pr"] = pr_number result["last_terminal_pr"] = pr_number
result["last_terminal_action"] = action result["last_terminal_action"] = action
result["locked_head_sha"] = locked_head
if pr_number is None: if pr_number is None:
result["reasons"].append( result["reasons"].append(
@@ -332,46 +165,25 @@ def assess_stale_review_decision_lock(
return result return result
live = classify_pr_live_state(pr_live) 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( result.update(
{ {
"pr_state": live["pr_state"], "pr_state": live["pr_state"],
"pr_merged": live["pr_merged"], "pr_merged": live["pr_merged"],
"pr_merged_or_closed": live["pr_merged_or_closed"], "pr_merged_or_closed": live["pr_merged_or_closed"],
"merge_commit_sha": live["merge_commit_sha"], "merge_commit_sha": live["merge_commit_sha"],
"current_pr_head_sha": current_head,
} }
) )
if not live["pr_merged_or_closed"]: if not live["pr_merged_or_closed"]:
stale_by_head = bool( result["reasons"].append(
locked_head and current_head and not heads_equal(locked_head, current_head) f"terminal review mutation on open PR #{pr_number} is still active "
f"({action}); #332 hard-stop remains — cleanup forbidden (#594)"
) )
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 return result
# Merged or closed: lock is moot. Same-PR merge continuation is impossible. # Merged or closed: lock is moot. Same-PR merge continuation is impossible.
result["is_moot"] = True result["is_moot"] = True
result["cleanup_allowed"] = True result["cleanup_allowed"] = True
result["stale_by_head"] = False
result["fresh_review_on_current_head_allowed"] = False
result["reasons"].append( result["reasons"].append(
f"terminal mutation ({action} on PR #{pr_number}) is moot: PR is " f"terminal mutation ({action} on PR #{pr_number}) is moot: PR is "
f"{'merged' if live['pr_merged'] else 'closed'}; canonical cleanup allowed (#594)" f"{'merged' if live['pr_merged'] else 'closed'}; canonical cleanup allowed (#594)"
-11
View File
@@ -139,17 +139,6 @@ 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",
+1
View File
@@ -275,6 +275,7 @@ def main():
servers[name], servers[name],
required_tool=REQUIRED_NAMESPACE_TOOLS.get(name), required_tool=REQUIRED_NAMESPACE_TOOLS.get(name),
config_path=config_path, config_path=config_path,
probe_source="offline_spawn",
): ):
failed = True failed = True
else: else:
-366
View File
@@ -1,366 +0,0 @@
"""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
@@ -1,824 +0,0 @@
"""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()
@@ -1,397 +0,0 @@
"""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()