Compare commits

..
4 Commits
Author SHA1 Message Date
jcwalker3 d7e69fbe77 Merge branch 'master' into feat/issue-639-webui-system-health-dashboard 2026-07-24 02:07:53 -05:00
sysadmin f80e3b33b0 Merge remote-tracking branch 'prgs/master' into feat/issue-639-webui-system-health-dashboard 2026-07-23 21:14:50 -04:00
sysadminandClaude Opus 4.8 edd5f813b2 Merge master into feat/issue-639-webui-system-health-dashboard
Resolve the #638 shell landing against the #639 dashboard:

- webui/layout.py: drop the flat NAV_ITEMS tuple in favor of master's
  grouped NAV_GROUPS nav-config module.
- webui/nav.py: register /system-health as a live item in the Health
  group, satisfying issue #639 AC5 through the canonical nav source.
- docs/webui-local-dev.md: keep both additive sections (#638 shell and
  #639 dashboard).
- tests/test_webui_system_health_dashboard.py: assert the nav entry via
  iter_nav_items() instead of the removed NAV_ITEMS tuple.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-23 19:56:57 -04:00
sysadminandClaude Opus 4.8 ecda200180 feat(webui): system-health dashboard (Closes #639)
Phase 1 child of the Web Console epic #631. Adds the operator-facing
system-health dashboard on top of the read-only system-health API landed
by #634, so runtime problems are visible on a surface instead of being
discovered late through failed LLM sessions.

- webui/system_health_views.py (new): renders the SystemHealthSnapshot as
  readiness, stale-runtime parity, version/uptime, dependency, MCP
  namespace, probe-error, and recovery cards.
- webui/app.py: GET /system-health, sharing load_system_health() with the
  JSON API so page and API cannot disagree. ?deep=1 behaves as on the API.
- webui/layout.py: nav entry and health card/badge styles.
- tests/test_webui_system_health_dashboard.py (new, 26 cases).
- docs/webui-local-dev.md: route, field authority, and redaction split.

Readiness honesty is preserved from the API: ready and readiness_complete
render separately, a probe that did not run is listed under "Not probed"
rather than counted healthy, and mutation safety is never claimed when the
runtime is stale or parity is indeterminate.

Redaction is split by field kind. Free text (probe details, reasons, probe
errors) passes through system_health.redact. Structured fields (commit
SHAs, probe names, statuses, timestamps) are HTML-escaped only: redact's
opaque-token rule matches any run of 32 or more characters, so routing a
40-character git SHA through it rendered "[redacted]" and blanked the
parity evidence the page exists to show.

Non-goals honored: no restart or reload controls (Phase 2, #642), no
manual process-kill guidance (#630). Read-only throughout.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-23 16:08:36 -04:00
18 changed files with 753 additions and 2571 deletions
-84
View File
@@ -525,90 +525,6 @@ def assess_ownership_record_activity(record: dict[str, Any]) -> dict[str, Any]:
} }
# Reviewer-lease reclaim is only reachable from a non-live (expired/stale) lease.
_RECLAIMABLE_REVIEWER_STATUSES = _EXPIRED_STATUSES | _STALE_STATUSES
def is_active_ownership_status(status: str | None) -> bool:
"""True when *status* denotes live/active ownership of a branch (#855).
Used to decide whether a *competing* active claimant still uses a branch
when weighing an expired reviewer lease for reclaim. Expired, stale,
released, and terminal statuses are not active.
"""
return _norm_str(status).lower() in _ACTIVE_OWNERSHIP_STATUSES
def assess_expired_reviewer_lease_reclaim(
*,
role: str,
status: str,
pr_merged: bool | None,
owner_pid_alive: bool | None,
competing_active_claimant: bool | None,
) -> dict[str, Any]:
"""Decide, explicitly and fail-closed, whether an expired reviewer lease
may stop protecting an already-merged branch (#855 AC4).
An expired reviewer lease should not protect a merged branch forever once
its work is done and no live claimant remains. Reclaim is permitted only
when **every** condition below is provably satisfied; any unknown
(``None``) or contrary value keeps the lease protective:
- the lease is a ``reviewer`` lease (author/merger/controller/reconciler
leases are out of scope and always keep protecting);
- its status is expired or stale (never an active/live lease);
- the PR is proven merged (``pr_merged is True``);
- the lease owner process is proven dead (``owner_pid_alive is False``);
- no competing active claimant uses the branch
(``competing_active_claimant is False``).
Returns a decision dict with ``reclaim_allowed`` and, when refused, the
fail-closed ``reasons``. The reasons never contain secrets — only the
role, the status, and which condition was unproven.
"""
reasons: list[str] = []
normalized_role = _norm_str(role).lower()
normalized_status = _norm_str(status).lower()
if normalized_role != "reviewer":
reasons.append(
f"lease role '{normalized_role or 'unknown'}' is not a reviewer "
"lease; expired-reviewer reclaim does not apply"
)
if normalized_status not in _RECLAIMABLE_REVIEWER_STATUSES:
reasons.append(
f"lease status '{normalized_status or 'unknown'}' is not expired "
"or stale; only a non-live reviewer lease may be reclaimed"
)
if pr_merged is not True:
reasons.append(
"PR merged state is not proven true; reclaim requires an "
"already-merged PR (fail closed)"
)
if owner_pid_alive is not False:
reasons.append(
"lease owner process liveness is not proven dead; a live owner "
"still protects the branch (fail closed)"
)
if competing_active_claimant is not False:
reasons.append(
"a competing active claimant may still use the branch; reclaim "
"requires no other active ownership (fail closed)"
)
allowed = not reasons
return {
"reclaim_allowed": allowed,
"role": normalized_role,
"status": normalized_status,
"decision": (
"reclaim_expired_reviewer_lease" if allowed else "keep_protecting"
),
"reasons": [] if allowed else reasons,
}
def assess_active_branch_ownership( def assess_active_branch_ownership(
*, *,
remote: str, remote: str,
-223
View File
@@ -1,223 +0,0 @@
# ADR: MCP restart governance and authorization policy
- **Status:** Accepted (policy effective immediately for LLM and operator sessions; enforcement tooling may lag)
- **Date:** 2026-07-23
- **Tracking issue:** [#656](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/656)
- **Policy version:** `restart-governance/v1`
- **Related:**
- Umbrella: [#655](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/655) — governed MCP restart coordination and zero-disruption recovery
- Vision: [#652](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/652) — MCP Control Plane Web Console product vision (§A system health and process control)
- Roadmap: [#653](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/653) — Control Plane Web Console phased delivery (Phase 2 restart controls)
- Contamination guard: [#630](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/630) — blocks manual process-kill recovery
- Console restart UX: [#642](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/642) — sanctioned restart and graceful reload
- Existing restart / reconnect paths to inventory: [#591](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/591) — auto-restart on master advance (closed); [#584](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/584) — host auto-reconnect on transport flap
- Stable-control runtime split: `docs/architecture/mcp-stable-control-runtime-policy-adr.md` (#615)
- Client-namespace health: `docs/mcp-namespace-health.md` (#543)
- Reconnect-only EOF recovery: `docs/mcp-namespace-eof-recovery.md`
## 1. Context
The Gitea MCP server is the **control plane** for real issue and PR mutations
(create, comment, lock, review, merge, reconcile). The same process serves every
role namespace (`gitea-author`, `gitea-reviewer`, `gitea-merger`,
`gitea-reconciler`, `gitea-controller`) and holds the in-memory capability-gate
code loaded at startup.
Restarting that process is destructive to concurrent work:
- It resets every session's identity, preflight, and capability-lease binding.
- It can interrupt a mutation mid-critical-section (a lock acquire, a review
submit, a merge), leaving durable state half-written.
- Relaunching from the wrong checkout or worktree silently changes which code
the control plane runs, defeating master-parity gates (#420 / #615).
Today there is **no durable written policy** stating who may restart MCP, under
what conditions, that restart is a last resort, and how controller approval,
automated safety gates, and break-glass interact. Operators and LLM sessions
therefore invent restart behavior ad hoc, which makes concurrent multi-role work
unsafe. #630 and #642 need this policy as their backbone.
This ADR defines that policy. It does **not** implement coordinator code or HA
multi-instance restart (those are later children of #655).
## 2. Decision
### 2.1 v1 decision (recorded)
**Restart authority in v1 is `controller approval + automated safety gates`.**
A restart of the stable control runtime is authorized only when **both** hold:
1. A **controller** role explicitly approves the restart, recording an audit
entry (who, why, scope, affected sessions), **and**
2. The **automated safety gates** pass: a completed drain acknowledgement (no
affected session is mid-critical-section) or a declared break-glass incident
(§2.5).
Quorum among multiple controllers is **not** required day-one. It is deferred
unless a later investigation (tracked under #653) proves single-controller
approval is insufficient. This ADR records the v1 decision so enforcement code
(#630) has a fixed target; changing it requires a superseding ADR.
### 2.2 Restart is a last resort — the recovery ladder
Restart is the **last** rung. Before any restart, exhaust the narrower
recoveries, in order:
1. **Reconnect** the IDE/client MCP namespace (transport EOF, `client is
closing: EOF`, transient `#584` flap). No process change. See
`docs/mcp-namespace-eof-recovery.md`.
2. **Refresh / rebind** the session workspace: re-run `gitea_whoami`,
`gitea_resolve_task_capability`, and pass an explicit validated
`worktree_path`. Fixes stale session context without touching the process.
3. **Scoped restart** of a single misbehaving namespace/service (where the
deployment supports per-service restart) rather than the whole control plane.
4. **Full restart** of the stable control runtime process — operator-owned,
controller-approved, drained.
5. **Host / infrastructure restart** — the broadest action; same authorization
as a full restart plus infrastructure ownership.
A session **must** try rungs 12 and record why they were insufficient before
requesting a restart at rung 3 or above. Skipping straight to restart is a
policy violation.
### 2.3 Authorization matrix
| Role | Reconnect (1) | Refresh/rebind (2) | Scoped restart (3) | Full restart (4) | Host restart (5) |
|---|---|---|---|---|---|
| **author** | self | self | request only | **forbidden** | forbidden |
| **reviewer** | self | self | request only | **forbidden** | forbidden |
| **merger** | self | self | request only | **forbidden** | forbidden |
| **reconciler** | self | self | request only | **forbidden** | forbidden |
| **controller** | self | self | **approve** (+gates) | **approve** (+gates) | request to operator |
| **operator** | self | self | execute (controller-approved) | execute (controller-approved) | execute (controller-approved) |
| **admin** | self | self | execute | execute | execute (break-glass) |
Legend: *self* = may perform for its own client session; *request only* = may
raise a restart request but not authorize or execute it; *approve* = may
authorize under §2.1 gates; *execute* = may perform the process action after the
authorization is recorded.
Key invariants:
- **No LLM worker role (author/reviewer/merger/reconciler) may perform or
authorize a full or host restart.** They may only reconnect/rebind their own
client and file a restart request.
- **Controller approval authorizes; operator/admin executes.** The approving
controller and the executing operator may be the same human, but both the
approval and the execution are audited.
- Privileged process actions (full restart, host restart) are reserved to
**operator/admin**, never to an automated worker.
### 2.4 Approved conditions
A restart at rung 3+ is approved only under one of these recorded conditions:
- **No affected sessions:** the control plane has no live session that would be
interrupted (verified, not assumed).
- **Full drain acknowledged:** every affected session has drained
(no open critical section — no held mutation lease mid-write) and the drain is
acknowledged in the audit record.
- **Controller + gates:** controller approval plus passing automated safety
gates (§2.1), the standard v1 path.
- **Quorum:** not required in v1; reserved for a future superseding ADR.
- **Break-glass:** an incident-backed emergency exception (§2.5).
Restart **never** bypasses mutation gates mid-critical-section. Drain before
restart is mandatory except under break-glass with a declared incident.
### 2.5 Break-glass
Break-glass is a **separate, narrower** authorization path for emergencies where
the normal drain-and-approve path cannot complete (e.g. the control plane is
wedged and cannot drain).
Break-glass conditions:
- A declared incident record exists (id, timestamp, declarer) **before** the
action.
- The action is taken by **operator or admin** authority only — never by an LLM
worker role, and never unilaterally by an operator with active peers when a
controller is reachable.
- The scope is the minimum necessary rung of the ladder.
- A **mandatory post-hoc audit** entry is filed: what was restarted, why the
normal path was impossible, which sessions were affected, and the incident id.
Break-glass suspends the drain requirement, not the audit requirement.
### 2.6 Explicit prohibitions
- **A unilateral LLM or operator full restart while active peer sessions
exist is forbidden.** An LLM worker role must not kill, restart, or relaunch
the MCP process; a lone operator must not full-restart over live peer work
without controller approval or a break-glass incident.
- Process-kill recovery is forbidden as a routine tool (#630). This ADR does not
introduce a kill path.
- Ambiguous policy state **denies** restart (§4).
## 3. Security requirements
- Full restart and host restart are **privileged**; only operator/admin execute
them, only after a controller approval or break-glass incident is recorded.
- Break-glass is a distinct authorization path with its own audit mandate; it is
never the default and never silent.
- **Every approval and every restart action is audited** (who approved, who
executed, scope, affected sessions, condition, policy version). No restart is
authorized without a durable audit entry.
## 4. Failure behavior
**Ambiguous policy → deny restart.** If it cannot be established that a
restart is authorized under §2 — unknown affected-session state, missing
controller approval, absent break-glass incident, or an unclassifiable request —
the safe action is to **refuse** the restart and stop with a recovery report,
never to restart on assumption.
## 5. Policy IDs (for enforcement code)
Enforcement code — the restart coordinator (a later child of #655), the #630
contamination guard, and the #642 console restart UX — binds to these stable
policy identifiers rather than to prose:
| Policy ID | Statement |
|---|---|
| `RG-01` | Restart is last resort; rungs 12 must be tried and recorded first (§2.2). |
| `RG-02` | v1 authority = controller approval + automated safety gates (§2.1). |
| `RG-03` | No LLM worker role performs or authorizes full/host restart (§2.3). |
| `RG-04` | Full/host restart executed by operator/admin only, post approval (§2.3). |
| `RG-05` | Drain before restart is mandatory except break-glass with incident (§2.4). |
| `RG-06` | Break-glass requires a pre-declared incident and post-hoc audit (§2.5). |
| `RG-07` | Unilateral LLM/operator full restart with active peers is forbidden (§2.6). |
| `RG-08` | Ambiguous policy state denies restart (§4). |
The `restart-governance/v1` **policy version** field is emitted on future
restart audit events so approvals can be reconciled against the policy revision
in force.
## 6. Dogfooding
Gitea-Tools governs its own MCP control plane by this policy. Author, reviewer,
merger, and reconciler sessions operating on this repository use the recovery
ladder (§2.2) — reconnect and rebind, never self-restart — and any real restart
of the Gitea-Tools stable control runtime follows the controller-approval +
drain path defined here.
## 7. Acceptance and cross-links
This ADR is the authoritative restart-governance policy. It **must** stay
cross-linked from the safety model and the web-console deployment boundary:
- `docs/safety-model.md` § Process restart governance references this ADR.
- `docs/webui-deployment.md` references this ADR for restart/reload disposition.
It is linked to its issue lineage — umbrella **#655**, vision **#652**, roadmap
**#653**, contamination guard **#630**, and console restart UX **#642** — in
§ Related above.
## 8. Non-goals
- Implementing the restart coordinator or approval state machine (#630, later
children of #655).
- Implementing HA multi-instance restart or quorum machinery.
- Introducing any process-kill or auto-restart tool; existing auto-restart
behavior must be inventoried before any new restart tool is enabled.
-14
View File
@@ -46,17 +46,3 @@ If shell helpers are unavailable and MCP commit cannot run, stop with a recovery
report (restart session, clear hung terminals, use MCP-native commit). See report (restart session, clear hung terminals, use MCP-native commit). See
[`llm-workflow-runbooks.md`](llm-workflow-runbooks.md) § MCP-native commit path [`llm-workflow-runbooks.md`](llm-workflow-runbooks.md) § MCP-native commit path
(#260) and agent temp artifact cleanup (#261). (#260) and agent temp artifact cleanup (#261).
## 7. Process restart governance
Restarting the MCP control-plane process is destructive to concurrent multi-role
work and is governed by a dedicated policy. Restart is a **last resort** behind
narrower recoveries (reconnect, rebind), full/host restart is reserved to
operator/admin under **controller approval + automated safety gates**, a
unilateral LLM or operator full restart with active peers is **forbidden**, and
ambiguous policy state **denies** restart. Break-glass is a separate,
incident-backed path with a mandatory audit.
See [`architecture/mcp-restart-governance.md`](architecture/mcp-restart-governance.md)
(#656) for the authorization matrix, the recovery ladder, break-glass
conditions, and the `RG-01``RG-08` policy IDs.
-9
View File
@@ -55,15 +55,6 @@ shipped to the browser.
assumption paths, and the client-secret policy. Use it to verify an instance is assumption paths, and the client-secret policy. Use it to verify an instance is
configured for internal-only operation. configured for internal-only operation.
## Process restart / reload disposition
The console never exposes a restart or reload control; process restart of the
MCP control-plane runtime is governed separately. Restart is a last resort behind
reconnect/rebind, full restart is operator/admin-only under controller approval
plus safety gates, and break-glass is an incident-backed path. See
[`architecture/mcp-restart-governance.md`](architecture/mcp-restart-governance.md)
(#656).
## Non-goals (MVP) ## Non-goals (MVP)
- Full SSO or session login in the UI - Full SSO or session login in the UI
+32
View File
@@ -54,6 +54,7 @@ status, onboarding checklist state, and the fail-closed error payloads (#635).
| `/` | Home / operator overview | | `/` | Home / operator overview |
| `/health` | JSON liveness (`status`, `service`, `mode`, `timestamp`, `uptime_seconds`) | | `/health` | JSON liveness (`status`, `service`, `mode`, `timestamp`, `uptime_seconds`) |
| `/api/v1/system/health` | Structured read-only system health (#634) | | `/api/v1/system/health` | Structured read-only system health (#634) |
| `/system-health` | System-health dashboard — readiness, version/uptime, dependencies, MCP namespaces, stale-runtime parity (#639) |
| `/queue` | Live PR and issue queue dashboard (#429) | | `/queue` | Live PR and issue queue dashboard (#429) |
| `/api/queue` | JSON queue export with pagination metadata | | `/api/queue` | JSON queue export with pagination metadata |
| `/projects` | Project registry list with status and onboarding progress (#427, #635) | | `/projects` | Project registry list with status and onboarding progress (#427, #635) |
@@ -258,6 +259,37 @@ Not-yet-implemented surfaces (`/sessions`, `/inventory`, `/timeline`,
surfaces are backed by #636). Mutating methods on stub routes still fail closed surfaces are backed by #636). Mutating methods on stub routes still fail closed
with `read-only-mvp`. with `read-only-mvp`.
## System-health dashboard (#639)
`/system-health` renders the same snapshot the `/api/v1/system/health` API
returns, so the page and the API can never disagree. Cards: overall readiness,
stale-runtime parity, version and uptime, dependency probes, MCP namespaces,
probe errors (only when present), and recovery pointers. `?deep=1` opts into
the network probe exactly as the API does; the plain page load stays cheap.
Field authority and honesty rules:
* `ready` and `readiness_complete` are shown separately. A snapshot whose
required probes never ran is not the same as one that ran them and passed,
and the page never collapses the two into an unproven green.
* A probe that did not run appears under **Not probed**, never as healthy.
* `stale_runtime.mutation_safe` is displayed verbatim from the API. When the
runtime is stale, or when parity is indeterminate, the page warns and does
not claim mutation safety.
* MCP namespaces are reported `unproven`: the web process runs outside the
IDE-managed MCP client and cannot prove that path (#543).
Redaction is split by field kind. Free text — probe details, readiness and
parity reasons, probe errors — passes through `system_health.redact`.
Structured fields — commit SHAs, probe names, statuses, timestamps — are
HTML-escaped only, because `redact`'s opaque-token rule matches any run of 32
or more characters and would otherwise blank every 40-character git SHA, which
is precisely the evidence the parity view exists to show.
The dashboard is read-only: no restart, reload, or process-kill control. Those
arrive in Phase 2 (#642). Recovery guidance points at the sanctioned client
reconnect / operator restart path — never a manual daemon kill (#630).
## Deployment boundary (#435) ## Deployment boundary (#435)
MVP serves on loopback by default. Binding `0.0.0.0` or `::` is **refused** MVP serves on loopback by default. Binding `0.0.0.0` or `::` is **refused**
+12 -296
View File
@@ -2445,20 +2445,6 @@ def _evaluate_issue_lock_recovery(
descendant_sha=local_head, descendant_sha=local_head,
) )
# #871: the inverse of the #768 descendant relation — the *remote* head may
# have advanced past the local/recorded head via a sanctioned merge-based
# branch sync (``gitea_update_pr_branch_by_merge``) while the local worktree
# stayed put. Observe that provenance server-side so the assessor can prove
# it and nothing else. Probed only when the heads differ; never from any
# caller-supplied value.
sync_provenance: dict | None = None
if remote_head and local_head and remote_head != local_head:
sync_provenance = issue_lock_worktree.read_merge_sync_provenance(
worktree_path,
prior_head_sha=local_head,
synced_head_sha=remote_head,
)
# #772: with no remote branch there is no head to measure against, so the # #772: with no remote branch there is no head to measure against, so the
# base the branch was cut from is observed instead. Probed only in that # base the branch was cut from is observed instead. Probed only in that
# case, so the published path's evidence is untouched (#772 AC8). # case, so the published path's evidence is untouched (#772 AC8).
@@ -2501,7 +2487,6 @@ def _evaluate_issue_lock_recovery(
remote_branch_exists=remote_branch_exists, remote_branch_exists=remote_branch_exists,
recorded_base_sha=recorded_base, recorded_base_sha=recorded_base,
base_ancestry=base_ancestry, base_ancestry=base_ancestry,
sync_provenance=sync_provenance,
) )
@@ -11131,9 +11116,6 @@ def _collect_branch_ownership_records(
""" """
records: list[dict] = [] records: list[dict] = []
inventory_error = False inventory_error = False
# #855 AC4: expired/stale reviewer-lease records eligible for an explicit
# reclaim decision, evaluated after the full ownership inventory is built.
reviewer_reclaim_candidates: list[tuple[dict, bool | None]] = []
target_branch = (branch or "").strip() target_branch = (branch or "").strip()
if not target_branch: if not target_branch:
return {"records": records, "inventory_error": False} return {"records": records, "inventory_error": False}
@@ -11282,27 +11264,14 @@ def _collect_branch_ownership_records(
else: else:
status = freshness_status status = freshness_status
reclaim_allowed = False reclaim_allowed = False
rec = _base_rec( records.append(
_base_rec(
category=category, category=category,
status=status, status=status,
reclaim_allowed=reclaim_allowed, reclaim_allowed=reclaim_allowed,
role=role, role=role,
host=lease_host or host_n or host, host=lease_host or host_n or host,
) )
records.append(rec)
# #855 AC4: a reviewer lease that is expired/stale (its owner
# gone) becomes a candidate for an explicit, fail-closed
# reclaim decision made once the full inventory is known.
if (
role == "reviewer"
and status
in branch_cleanup_guard._RECLAIMABLE_REVIEWER_STATUSES
):
owner_alive = (
fr.get("owner_pid_alive") if isinstance(fr, dict) else None
)
reviewer_reclaim_candidates.append(
(rec, owner_alive if isinstance(owner_alive, bool) else None)
) )
except Exception: except Exception:
# O1: fail closed on control-plane inventory errors. # O1: fail closed on control-plane inventory errors.
@@ -11374,44 +11343,6 @@ def _collect_branch_ownership_records(
) )
) )
# #855 AC4: decide, explicitly and fail-closed, whether any expired/stale
# reviewer lease may stop protecting an already-merged branch. This runs
# only after the full ownership inventory is built, so a competing active
# claimant (an active lease, author session, worktree binding, or active
# reviewer comment lease) is visible. An inventory failure keeps every
# reclaim candidate protective (reclaim_allowed stays False).
if reviewer_reclaim_candidates and not inventory_error:
pr_merged_state: bool | None = None
if pr_number is not None and auth and base_api:
try:
pr_live = api_request(
"GET", f"{base_api}/pulls/{int(pr_number)}", auth
)
if isinstance(pr_live, dict) and pr_live:
pr_merged_state = bool(
pr_live.get("merged") or pr_live.get("merged_at")
)
except Exception:
# Unknown merged state fails closed (candidate stays protective).
pr_merged_state = None
for cand_rec, owner_alive in reviewer_reclaim_candidates:
competing = any(
other is not cand_rec
and branch_cleanup_guard.is_active_ownership_status(
other.get("status")
)
for other in records
)
decision = branch_cleanup_guard.assess_expired_reviewer_lease_reclaim(
role=str(cand_rec.get("role")),
status=str(cand_rec.get("status")),
pr_merged=pr_merged_state,
owner_pid_alive=owner_alive,
competing_active_claimant=competing,
)
cand_rec["reclaim_allowed"] = decision["reclaim_allowed"]
cand_rec["reclaim_decision"] = decision["decision"]
return {"records": records, "inventory_error": inventory_error} return {"records": records, "inventory_error": inventory_error}
@@ -11474,7 +11405,6 @@ def gitea_reconcile_merged_cleanups(
dry_run: bool = True, dry_run: bool = True,
execute_confirmed: bool = False, execute_confirmed: bool = False,
limit: int = 50, limit: int = 50,
pr_number: int | None = None,
remote: str = "dadeschools", remote: str = "dadeschools",
host: str | None = None, host: str | None = None,
org: str | None = None, org: str | None = None,
@@ -11485,11 +11415,7 @@ def gitea_reconcile_merged_cleanups(
Args: Args:
dry_run: Defaults to True. When True, only builds the reconciliation report. dry_run: Defaults to True. When True, only builds the reconciliation report.
execute_confirmed: Must be True when dry_run=False. execute_confirmed: Must be True when dry_run=False.
limit: Max number of closed PRs to inspect (batch mode only; ignored when limit: Max number of closed PRs to inspect.
``pr_number`` is set).
pr_number: Optional exact merged PR selector (#855). When set, only that
PR is assessed/acted on (fail closed if missing, unmerged, or
ambiguous). When omitted, existing batch behaviour is preserved.
remote: Known Gitea instance ('dadeschools' or 'prgs'). remote: Known Gitea instance ('dadeschools' or 'prgs').
host: Override the Gitea host. host: Override the Gitea host.
org: Override the owner/organization. org: Override the owner/organization.
@@ -11524,118 +11450,9 @@ def gitea_reconcile_merged_cleanups(
"audit_phase": audit_reconciliation_mode.current_phase(), "audit_phase": audit_reconciliation_mode.current_phase(),
} }
# #855: optional exact PR pin. Fail closed before any inventory mutation.
exact_pr: int | None = None
if pr_number is not None:
try:
exact_pr = int(pr_number)
except (TypeError, ValueError):
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": "exact_pr",
"selected_pr_number": pr_number,
"reasons": [
f"pr_number={pr_number!r} is not a valid integer "
"(fail closed; no mutation)"
],
"blocker_kind": "invalid_pr_number",
}
if exact_pr <= 0:
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": "exact_pr",
"selected_pr_number": exact_pr,
"reasons": [
f"pr_number={exact_pr} must be a positive integer "
"(fail closed; no mutation)"
],
"blocker_kind": "invalid_pr_number",
}
h, o, r = _resolve(remote, host, org, repo) h, o, r = _resolve(remote, host, org, repo)
auth = _auth(h) auth = _auth(h)
base = repo_api_url(h, o, r) base = repo_api_url(h, o, r)
selection_mode = "batch"
closed_prs: list[dict] = []
open_prs: list[dict] = []
if exact_pr is not None:
selection_mode = "exact_pr"
try:
pr_live = api_request("GET", f"{base}/pulls/{exact_pr}", auth)
except Exception as exc:
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": selection_mode,
"selected_pr_number": exact_pr,
"reasons": [
f"PR #{exact_pr} could not be uniquely resolved "
f"(fail closed; no mutation): {_redact(str(exc))}"
],
"blocker_kind": "pr_unresolvable",
}
if not isinstance(pr_live, dict) or not pr_live:
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": selection_mode,
"selected_pr_number": exact_pr,
"reasons": [
f"PR #{exact_pr} could not be uniquely resolved "
"(empty response; fail closed; no mutation)"
],
"blocker_kind": "pr_unresolvable",
}
live_number = pr_live.get("number")
try:
live_number_int = int(live_number) if live_number is not None else None
except (TypeError, ValueError):
live_number_int = None
if live_number_int != exact_pr:
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": selection_mode,
"selected_pr_number": exact_pr,
"reasons": [
f"PR #{exact_pr} resolution is ambiguous or mismatched "
f"(live number={live_number!r}; fail closed; no mutation)"
],
"blocker_kind": "pr_ambiguous",
}
if not (pr_live.get("merged") or pr_live.get("merged_at")):
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": selection_mode,
"selected_pr_number": exact_pr,
"reasons": [
f"PR #{exact_pr} is not merged "
"(exact-target cleanup requires a merged PR; "
"fail closed; no mutation)"
],
"blocker_kind": "pr_not_merged",
}
closed_prs = [pr_live]
# Exact mode still needs open heads for remote-delete safety gates.
open_prs = api_get_all(f"{base}/pulls?state=open", auth)
else:
# Preserve historical call order (closed then open) for batch callers/tests.
closed_prs = api_get_all(f"{base}/pulls?state=closed", auth, limit=limit) closed_prs = api_get_all(f"{base}/pulls?state=closed", auth, limit=limit)
open_prs = api_get_all(f"{base}/pulls?state=open", auth) open_prs = api_get_all(f"{base}/pulls?state=open", auth)
@@ -11664,13 +11481,6 @@ def gitea_reconcile_merged_cleanups(
scratch_candidates = merged_cleanup_reconcile.discover_reviewer_scratch_worktrees( scratch_candidates = merged_cleanup_reconcile.discover_reviewer_scratch_worktrees(
_canonical_local_git_root() _canonical_local_git_root()
) )
# #855: exact-target never inventories or mutates foreign PR scratch trees.
if exact_pr is not None:
scratch_candidates = [
s
for s in scratch_candidates
if int(s.get("pr_number") or 0) == int(exact_pr)
]
active_reviewer_leases: dict[int, bool] = {} active_reviewer_leases: dict[int, bool] = {}
pr_states: dict[int, dict] = {} pr_states: dict[int, dict] = {}
for scratch in scratch_candidates: for scratch in scratch_candidates:
@@ -11705,33 +11515,6 @@ def gitea_reconcile_merged_cleanups(
active_reviewer_leases=active_reviewer_leases, active_reviewer_leases=active_reviewer_leases,
pr_states=pr_states, pr_states=pr_states,
) )
report["selection_mode"] = selection_mode
if exact_pr is not None:
report["selected_pr_number"] = exact_pr
# Fail closed if exact pin somehow produced other or zero entries.
entries = list(report.get("entries") or [])
entry_numbers = []
for entry in entries:
try:
entry_numbers.append(int(entry.get("pr_number")))
except (TypeError, ValueError):
entry_numbers.append(entry.get("pr_number"))
if entry_numbers != [exact_pr]:
return {
"success": False,
"performed": False,
"executed": False,
"dry_run": bool(dry_run),
"selection_mode": selection_mode,
"selected_pr_number": exact_pr,
"reasons": [
f"exact PR #{exact_pr} selection produced unexpected "
f"candidate set {entry_numbers!r} "
"(fail closed; no mutation)"
],
"blocker_kind": "exact_selection_mismatch",
"entries": entries,
}
if dry_run: if dry_run:
report["dry_run"] = True report["dry_run"] = True
@@ -19106,59 +18889,10 @@ def gitea_update_pr_branch_by_merge(
prepared_verdict_head_sha=live_pr_head, prepared_verdict_head_sha=live_pr_head,
) )
# #871: the remote head is now advanced; the durable linked-issue lock must return {
# be advanced with it, or a later dead-session recovery can never prove "success": True,
# ownership at the new head. This runs AFTER the successful remote update, so
# a failure here is a *partial* lifecycle failure — the remote moved but the
# durable state did not — and must never be reported as a full success.
claimant = _work_lease_claimant(h)
matched_issue = ownership.get("matched_issue")
synced_at = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
lock_refresh: dict = {
"refreshed": False,
"reasons": ["durable lock head refresh was not attempted"],
}
if new_head and matched_issue and source_branch and (wt or None):
try:
lock_refresh = issue_lock_store.apply_durable_lock_head_refresh(
remote=remote,
org=o,
repo=r,
issue_number=int(matched_issue),
branch_name=source_branch,
worktree_path=wt,
pr_number=pr_number,
identity=claimant.get("username"),
profile=claimant.get("profile"),
current_pid=os.getpid(),
expected_old_head=live_pr_head,
new_head=new_head,
synced_at=synced_at,
base_head=live_base_head,
)
except Exception as exc:
lock_refresh = {
"refreshed": False,
"reasons": [
f"durable lock head refresh raised (fail closed): {_redact(str(exc))}"
],
}
else:
lock_refresh = {
"refreshed": False,
"reasons": [
"durable lock head refresh could not run: missing new head, "
"linked issue, source branch, or worktree binding"
],
}
durable_refreshed = bool(lock_refresh.get("refreshed"))
base_result = {
"performed": True, "performed": True,
"mutation_allowed": True, "mutation_allowed": True,
"durable_lock_refreshed": durable_refreshed,
"durable_lock_refresh": lock_refresh,
"fully_synchronized": durable_refreshed,
"style": "merge", "style": "merge",
"force_push": False, "force_push": False,
"rebase": False, "rebase": False,
@@ -19180,37 +18914,19 @@ def gitea_update_pr_branch_by_merge(
"prepared_verdict_invalidated": transition.get( "prepared_verdict_invalidated": transition.get(
"prepared_verdict_invalidated" "prepared_verdict_invalidated"
), ),
"recommended_next_action": transition.get(
"recommended_next_action",
pr_sync_status.ACTION_FRESH_REVIEW_REQUIRED,
),
"transition": transition, "transition": transition,
"role_kind": role, "role_kind": role,
"profile_name": profile.get("profile_name"), "profile_name": profile.get("profile_name"),
"worktree_path": wt or None, "worktree_path": wt or None,
"reasons": list(transition.get("reasons") or []) + [
"update-by-merge completed via native Gitea API (style=merge only)"
],
} }
if durable_refreshed:
base_result["success"] = True
base_result["recommended_next_action"] = transition.get(
"recommended_next_action",
pr_sync_status.ACTION_FRESH_REVIEW_REQUIRED,
)
base_result["reasons"] = list(transition.get("reasons") or []) + [
"update-by-merge completed via native Gitea API (style=merge only)",
f"durable linked-issue lock #{matched_issue} head refreshed to "
f"{new_head} (verified by read-after-write)",
]
return base_result
# Partial lifecycle failure: the remote advanced but the durable lock did
# not. Do NOT report a fully successful synchronization (#871).
base_result["success"] = False
base_result["partial_lifecycle_failure"] = True
base_result["recommended_next_action"] = pr_sync_status.ACTION_BLOCKED
base_result["reasons"] = list(lock_refresh.get("reasons") or []) + [
f"PARTIAL LIFECYCLE FAILURE: PR #{pr_number} remote head advanced to "
f"{new_head} but the durable linked-issue lock head was not refreshed; "
"the synchronization is NOT complete",
]
return base_result
@mcp.tool() @mcp.tool()
def gitea_assess_conflict_fix_push( def gitea_assess_conflict_fix_push(
+5 -127
View File
@@ -85,12 +85,6 @@ HEAD_RELATION_STRICT_DESCENDANT = "strict_descendant"
# #772: an unpublished claim has no recorded head to compare against at all, so # #772: an unpublished claim has no recorded head to compare against at all, so
# its head is measured against the base the branch was cut from instead. # its head is measured against the base the branch was cut from instead.
HEAD_RELATION_DESCENDS_FROM_BASE = "descends_from_recorded_base" HEAD_RELATION_DESCENDS_FROM_BASE = "descends_from_recorded_base"
# #871: the remote/PR head advanced *past* the recorded head via a sanctioned
# merge-based branch synchronization (``gitea_update_pr_branch_by_merge``) while
# the local worktree stayed at the recorded head. This is the inverse of the
# #768 descendant relation — here the *remote* strictly descends the local head,
# and only because a base was merged into the branch, proven server-side.
HEAD_RELATION_REMOTE_MERGE_SYNCED = "remote_merge_synced"
# Which body of evidence a recovery was decided on (#772 AC10). These are not # Which body of evidence a recovery was decided on (#772 AC10). These are not
# interchangeable: a published claim proves ownership against a remote/PR head, # interchangeable: a published claim proves ownership against a remote/PR head,
@@ -272,70 +266,6 @@ def _assess_base_descendancy(
] ]
def _assess_remote_merge_synced(
sync_provenance: Mapping[str, Any] | None,
*,
recorded_head: str,
remote_head: str,
) -> tuple[bool, list[str]]:
"""Did ``remote_head`` advance past ``recorded_head`` via a sanctioned
merge-based branch sync (#871)?
``sync_provenance`` is the server-side git observation from
``issue_lock_worktree.read_merge_sync_provenance``. Its own
``prior_head_sha`` / ``synced_head_sha`` are re-checked against the heads
this assessment is actually reasoning about, so an observation taken for some
other pair of commits — stale, mismatched, or hand-built — can never
authorize recovery. This is the inverse of ``_assess_strict_descendant``: the
recorded head is the ancestor and the *remote* head is the descendant, and it
is accepted only because the remote head is a base-into-branch merge that
preserved the branch mainline back to the recorded head.
Returns ``(proven, notes)``. Notes name the exact missing element so a
refused caller sees why, never a bare "unproven".
"""
if not isinstance(sync_provenance, Mapping):
return False, [
"no server-derived merge-sync provenance observation was available; a "
"remote head ahead of the recorded head cannot be accepted"
]
probe_prior = _text(sync_provenance.get("prior_head_sha"))
probe_synced = _text(sync_provenance.get("synced_head_sha"))
if probe_prior != recorded_head or probe_synced != remote_head:
return False, [
f"merge-sync observation covers {probe_prior or 'unknown'} -> "
f"{probe_synced or 'unknown'}, not the heads under assessment "
f"({recorded_head} -> {remote_head})"
]
if not sync_provenance.get("probe_ok"):
return False, (
list(sync_provenance.get("reasons") or [])
or ["merge-sync provenance probe did not complete; provenance unproven"]
)
if not sync_provenance.get("prior_is_ancestor"):
return False, [
f"recorded head {recorded_head} is not an ancestor of remote head "
f"{remote_head}; a rewritten or force-moved head cannot be recovered"
]
if not sync_provenance.get("is_merge_sync"):
return False, (
list(sync_provenance.get("reasons") or [])
or [
f"remote head {remote_head} is not a sanctioned merge-based sync "
f"of the base into the branch above {recorded_head}"
]
)
proof = _text(sync_provenance.get("proof")) or (
f"{remote_head} merged the base into the branch above {recorded_head}"
)
return True, [
f"remote head {remote_head} advanced past recorded head {recorded_head} "
f"via a sanctioned merge-based branch sync ({proof})"
]
def assess_dead_session_lock_recovery( def assess_dead_session_lock_recovery(
existing_lock: Mapping[str, Any] | None, existing_lock: Mapping[str, Any] | None,
*, *,
@@ -360,7 +290,6 @@ def assess_dead_session_lock_recovery(
remote_branch_exists: bool | None = None, remote_branch_exists: bool | None = None,
recorded_base_sha: str | None = None, recorded_base_sha: str | None = None,
base_ancestry: Mapping[str, Any] | None = None, base_ancestry: Mapping[str, Any] | None = None,
sync_provenance: Mapping[str, Any] | None = None,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""Decide whether a dead-session author lock may be natively recovered. """Decide whether a dead-session author lock may be natively recovered.
@@ -539,39 +468,19 @@ def assess_dead_session_lock_recovery(
if descends: if descends:
head_relation = HEAD_RELATION_STRICT_DESCENDANT head_relation = HEAD_RELATION_STRICT_DESCENDANT
ancestry_proof = notes[0] if notes else None ancestry_proof = notes[0] if notes else None
else:
# #871: the reverse relation — the remote head advanced past
# the recorded/local head via a sanctioned merge-based branch
# sync while the local worktree stayed put. Accepted only on
# server-proven merge-sync provenance, never a caller claim.
synced, sync_notes = _assess_remote_merge_synced(
sync_provenance,
recorded_head=local_head,
remote_head=remote_head,
)
if synced:
head_relation = HEAD_RELATION_REMOTE_MERGE_SYNCED
ancestry_proof = sync_notes[0] if sync_notes else None
else: else:
reasons.append( reasons.append(
f"local head {local_head} does not match remote branch " f"local head {local_head} does not match remote branch head "
f"head {remote_head}" f"{remote_head}"
) )
reasons.extend(notes) reasons.extend(notes)
reasons.extend(sync_notes)
evidence["recorded_base"] = recorded_base or None evidence["recorded_base"] = recorded_base or None
evidence["local_head"] = local_head or None evidence["local_head"] = local_head or None
evidence["remote_head"] = remote_head or None evidence["remote_head"] = remote_head or None
# ``recorded_head`` is the head recovery is being measured against; # ``recorded_head`` is the head recovery is being measured against;
# ``accepted_head`` is the head this recovery actually adopts. They differ # ``accepted_head`` is the head this recovery actually adopts. They differ
# only in the descendant case, and downstream gates need both (#768 AC2/AC7). # only in the descendant case, and downstream gates need both (#768 AC2/AC7).
# #871: in the merge-sync case the branch/PR already carries the synced
# remote head, so that is the head recovery adopts; the local worktree stays
# at the ancestor recorded head.
evidence["recorded_head"] = remote_head or None evidence["recorded_head"] = remote_head or None
if head_relation == HEAD_RELATION_REMOTE_MERGE_SYNCED:
evidence["accepted_head"] = remote_head or None
else:
evidence["accepted_head"] = local_head or None evidence["accepted_head"] = local_head or None
evidence["head_relation"] = head_relation evidence["head_relation"] = head_relation
evidence["ancestry_proof"] = ancestry_proof evidence["ancestry_proof"] = ancestry_proof
@@ -584,17 +493,12 @@ def assess_dead_session_lock_recovery(
# contradictory; re-stating it as a head mismatch would only obscure why. # contradictory; re-stating it as a head mismatch would only obscure why.
if not unpublished and local_head and pr_head != local_head: if not unpublished and local_head and pr_head != local_head:
# A descendant recovery has not been published yet, so the open PR # A descendant recovery has not been published yet, so the open PR
# legitimately still points at the recorded head. A merge-sync # legitimately still points at the recorded head. Any other
# recovery's PR legitimately sits at the advanced remote head. Any # disagreement is a real mismatch.
# other disagreement is a real mismatch.
if not ( if not (
head_relation == HEAD_RELATION_STRICT_DESCENDANT head_relation == HEAD_RELATION_STRICT_DESCENDANT
and remote_head and remote_head
and pr_head == remote_head and pr_head == remote_head
) and not (
head_relation == HEAD_RELATION_REMOTE_MERGE_SYNCED
and remote_head
and pr_head == remote_head
): ):
reasons.append( reasons.append(
f"open PR #{pr_number} head {pr_head} does not match local head " f"open PR #{pr_number} head {pr_head} does not match local head "
@@ -715,11 +619,7 @@ def assess_dead_session_lock_recovery(
) )
if ( if (
head_relation head_relation
in ( in (HEAD_RELATION_STRICT_DESCENDANT, HEAD_RELATION_DESCENDS_FROM_BASE)
HEAD_RELATION_STRICT_DESCENDANT,
HEAD_RELATION_DESCENDS_FROM_BASE,
HEAD_RELATION_REMOTE_MERGE_SYNCED,
)
and ancestry_proof and ancestry_proof
): ):
proof.append(ancestry_proof) proof.append(ancestry_proof)
@@ -797,16 +697,6 @@ def owning_pr_recovery_evidence(
return None return None
if accepted_head != local_head: if accepted_head != local_head:
return None return None
elif relation == HEAD_RELATION_REMOTE_MERGE_SYNCED:
# #871: the PR already sits at the advanced remote head; the local
# worktree is the ancestor the merge preserved. The head the open PR
# shows and the head recovery adopts are both the synced remote head.
if not remote_head or pr_head != remote_head:
return None
if accepted_head != remote_head:
return None
if not local_head or local_head == remote_head:
return None
else: else:
return None return None
try: try:
@@ -875,18 +765,6 @@ def recovered_owning_pr_from_lock(
return None return None
if not accepted_head or accepted_head == recorded_head: if not accepted_head or accepted_head == recorded_head:
return None return None
elif relation == HEAD_RELATION_REMOTE_MERGE_SYNCED:
# #871: PR sits at the advanced remote head, which is both the recorded
# measured-against head and the adopted head; the local worktree is the
# ancestor the merge preserved.
remote_head = _text(record.get("remote_head"))
local_head = _text(record.get("local_head"))
if not remote_head or pr_head != remote_head:
return None
if accepted_head and accepted_head != remote_head:
return None
if not local_head or local_head == remote_head:
return None
else: else:
return None return None
try: try:
-304
View File
@@ -738,307 +738,3 @@ def format_lock_proof(
elif released is False: elif released is False:
parts.append("lock retained") parts.append("lock retained")
return "; ".join(parts) return "; ".join(parts)
# ── #871: durable linked-issue lock head refresh after branch synchronization ──
_FULL_SHA_RE = re.compile(r"^[0-9a-f]{40}$", re.IGNORECASE)
# Provenance recorded on the lock when the head is refreshed by a sanctioned
# merge-based branch synchronization (``gitea_update_pr_branch_by_merge``).
LOCK_HEAD_REFRESH_PROVENANCE_MERGE_SYNC = "gitea_update_pr_branch_by_merge"
def _norm_sha(value: Any) -> str | None:
text = str(value or "").strip().lower()
return text if _FULL_SHA_RE.match(text) else None
def _lock_claimant_view(lock: dict[str, Any] | None) -> dict[str, Any]:
if not isinstance(lock, dict):
return {}
claimant = lock.get("claimant")
if not isinstance(claimant, dict):
lease = lock.get("work_lease")
claimant = lease.get("claimant") if isinstance(lease, dict) else None
return dict(claimant) if isinstance(claimant, dict) else {}
def assess_durable_lock_head_refresh(
existing_lock: dict[str, Any] | None,
*,
remote: str,
org: str,
repo: str,
issue_number: int,
branch_name: str,
worktree_path: str,
pr_number: int | None,
identity: str | None,
profile: str | None,
current_pid: int | None,
expected_old_head: str | None,
new_head: str | None,
base_head: str | None = None,
) -> dict[str, Any]:
"""Fail-closed assessment for refreshing a durable lock's recorded head (#871).
A successful ``gitea_update_pr_branch_by_merge`` advances the *remote* PR head
but must also advance the durable linked-issue lock so a later dead-session
recovery can prove ownership. This decides whether that refresh is permitted;
it mutates nothing.
Every element of durable ownership is re-verified against the persisted lock —
repository, issue, branch, worktree, claimant identity/profile, and the live
owning session — and the recorded head is compare-and-swapped: the lock's
currently recorded synced head (if any) must equal ``expected_old_head``, so a
lock whose head or provenance changed concurrently is never overwritten.
"""
reasons: list[str] = []
old = _norm_sha(expected_old_head)
new = _norm_sha(new_head)
evidence: dict[str, Any] = {
"issue_number": issue_number,
"branch_name": branch_name,
"worktree_path": worktree_path,
"pr_number": pr_number,
"expected_old_head": old,
"new_head": new,
"base_head": _norm_sha(base_head),
}
if not isinstance(existing_lock, dict) or not existing_lock:
reasons.append("no durable lock exists for this issue; nothing to refresh")
return {"allowed": False, "reasons": reasons, "evidence": evidence,
"expected_generation": 0}
lock = dict(existing_lock)
evidence["current_generation"] = lock_generation(lock)
if lock.get("issue_number") != issue_number:
reasons.append(
f"durable lock targets issue #{lock.get('issue_number')}, not "
f"#{issue_number}; refusing head refresh"
)
for field, expected in (("remote", remote), ("org", org), ("repo", repo)):
actual = str(lock.get(field) or "").strip()
if actual != str(expected or "").strip():
reasons.append(
f"lock {field} '{actual}' does not match requested "
f"'{str(expected or '').strip()}'"
)
locked_branch = str(lock.get("branch_name") or "").strip()
if locked_branch != str(branch_name or "").strip():
reasons.append(
f"lock branch '{locked_branch}' does not match requested "
f"'{str(branch_name or '').strip()}'"
)
locked_worktree = str(lock.get("worktree_path") or "").strip()
try:
same_wt = bool(locked_worktree) and bool(worktree_path) and (
os.path.realpath(locked_worktree) == os.path.realpath(worktree_path)
)
except OSError:
same_wt = locked_worktree == (worktree_path or "")
if not same_wt:
reasons.append(
f"lock worktree '{locked_worktree}' does not match declared "
f"'{str(worktree_path or '').strip()}'"
)
claimant = _lock_claimant_view(lock)
locked_identity = str(claimant.get("username") or "").strip()
locked_profile = str(claimant.get("profile") or "").strip()
if not locked_identity or not locked_profile:
reasons.append(
"durable lock does not record a claimant identity/profile; "
"ownership could not be proven for head refresh"
)
if not str(identity or "").strip() or not str(profile or "").strip():
reasons.append(
"active session identity/profile is unknown; ownership could not be "
"proven for head refresh"
)
if locked_identity and str(identity or "").strip() and locked_identity != str(identity).strip():
reasons.append(
f"lock claimant '{locked_identity}' does not match active identity "
f"'{str(identity).strip()}'"
)
if locked_profile and str(profile or "").strip() and locked_profile != str(profile).strip():
reasons.append(
f"lock profile '{locked_profile}' does not match active profile "
f"'{str(profile).strip()}'"
)
# The refresh is written by the LIVE owning author session. A refresh is not
# a recovery: the current process must be the recorded owner.
recorded_pid = lock.get("session_pid")
if recorded_pid is None:
recorded_pid = lock.get("pid")
evidence["recorded_pid"] = recorded_pid
evidence["current_pid"] = current_pid
if current_pid is None:
reasons.append("current session pid is unknown; cannot prove live ownership")
else:
try:
if recorded_pid is None or int(recorded_pid) != int(current_pid):
reasons.append(
f"durable lock is owned by pid {recorded_pid}, not the current "
f"session pid {current_pid}; head refresh requires the live owner"
)
except (TypeError, ValueError):
reasons.append(
"durable lock owner pid is malformed; cannot prove live ownership"
)
if not old:
reasons.append("expected_old_head is not a full 40-char hex SHA (fail closed)")
if not new:
reasons.append("new_head is not a full 40-char hex SHA (fail closed)")
if old and new and old == new:
reasons.append(
"new head equals the expected old head; a sync must advance the head"
)
# Compare-and-swap on the recorded head: if the lock already records a synced
# head it must be exactly the expected old head, else another sync moved it.
recorded_synced = _norm_sha(lock.get("synced_pr_head"))
evidence["recorded_synced_pr_head"] = recorded_synced
if recorded_synced is not None and old is not None and recorded_synced != old:
reasons.append(
f"durable lock already records synced head {recorded_synced}, not the "
f"expected old head {old}; a concurrent sync changed it (CAS fail closed)"
)
if reasons:
return {"allowed": False, "reasons": reasons, "evidence": evidence,
"expected_generation": lock_generation(lock)}
return {
"allowed": True,
"reasons": [
f"durable lock for issue #{issue_number} branch '{locked_branch}' is "
f"owned by the live session; refresh recorded head {old} -> {new}"
],
"evidence": evidence,
"expected_generation": lock_generation(lock),
}
def apply_durable_lock_head_refresh(
*,
remote: str,
org: str,
repo: str,
issue_number: int,
branch_name: str,
worktree_path: str,
pr_number: int | None,
identity: str | None,
profile: str | None,
current_pid: int | None,
expected_old_head: str | None,
new_head: str | None,
synced_at: str,
base_head: str | None = None,
provenance: str = LOCK_HEAD_REFRESH_PROVENANCE_MERGE_SYNC,
lock_dir: str | None = None,
) -> dict[str, Any]:
"""CAS-refresh the durable lock's recorded head after a branch sync (#871).
Reads the durable lock from disk, re-asserts ownership via
``assess_durable_lock_head_refresh``, and — only when permitted — writes the
new synced head through ``bind_session_lock`` with a generation compare-and-
swap. Then re-reads the lock and proves it records the complete new head
(read-after-write). Any failure at any step returns ``refreshed=False`` with
reasons; the caller must treat that as a partial lifecycle failure and never
report a fully successful synchronization.
"""
existing = load_issue_lock(
remote=remote, org=org, repo=repo, issue_number=issue_number, lock_dir=lock_dir
)
assessment = assess_durable_lock_head_refresh(
existing,
remote=remote,
org=org,
repo=repo,
issue_number=issue_number,
branch_name=branch_name,
worktree_path=worktree_path,
pr_number=pr_number,
identity=identity,
profile=profile,
current_pid=current_pid,
expected_old_head=expected_old_head,
new_head=new_head,
base_head=base_head,
)
result: dict[str, Any] = {
"refreshed": False,
"read_after_write_ok": False,
"prior_head": _norm_sha(expected_old_head),
"new_head": _norm_sha(new_head),
"reasons": list(assessment.get("reasons") or []),
"evidence": assessment.get("evidence"),
}
if not assessment.get("allowed"):
return result
new = _norm_sha(new_head)
old = _norm_sha(expected_old_head)
record = dict(existing or {})
sync_block = {
"last_synced_pr_head": new,
"prior_pr_head": old,
"base_head": _norm_sha(base_head),
"pr_number": pr_number,
"provenance": provenance,
"synced_at": synced_at,
"synced_by_pid": current_pid,
"synced_by": {
"username": str(identity or "").strip() or None,
"profile": str(profile or "").strip() or None,
},
}
record["synced_pr_head"] = new
record["branch_sync"] = sync_block
history = record.get("branch_sync_history")
if not isinstance(history, list):
history = []
history = list(history)
history.append(sync_block)
record["branch_sync_history"] = history
try:
bind_session_lock(
record,
lock_dir=lock_dir,
expected_generation=assessment.get("expected_generation"),
renewal_sanctioned=True,
)
except Exception as exc: # CAS miss or write failure — partial lifecycle failure
result["reasons"].append(
f"durable lock head refresh write failed (fail closed): {exc}"
)
return result
after = load_issue_lock(
remote=remote, org=org, repo=repo, issue_number=issue_number, lock_dir=lock_dir
)
after_head = _norm_sha((after or {}).get("synced_pr_head"))
result["lock_generation_after"] = lock_generation(after)
if after_head == new and new is not None:
result["refreshed"] = True
result["read_after_write_ok"] = True
result["reasons"].append(
f"durable lock recorded head refreshed to {new} and verified by "
"read-after-write"
)
else:
result["reasons"].append(
"read-after-write verification failed: durable lock does not record "
f"the new head {new} (found {after_head}); partial lifecycle failure"
)
return result
-169
View File
@@ -145,175 +145,6 @@ def read_head_ancestry(
return result return result
def read_merge_sync_provenance(
worktree_path: str,
*,
prior_head_sha: str | None,
synced_head_sha: str | None,
) -> dict:
"""Observe whether ``synced_head_sha`` is a sanctioned merge-based branch sync
that advanced the PR branch past ``prior_head_sha`` (#871).
``gitea_update_pr_branch_by_merge`` advances a PR branch by merging the base
branch *into* the branch (``POST /pulls/{n}/update?style=merge``). The result
is a merge commit ``M`` on the branch whose **first** parent is the prior
branch head and whose second parent is the base tip. When the owning session
then dies without the durable lock's recorded head being refreshed, the local
worktree still sits at ``prior_head_sha`` while the live PR head is ``M``.
Recovering that drift safely requires proving the remote head is *exactly*
such a merge-sync — not a rewrite, rebase, force-push, or an unrelated
commit. This is that server-side observation. It reports facts only; the
disposition lives in ``issue_lock_recovery``. Every field is read from git in
the declared worktree — nothing is supplied by, or reachable from, an MCP
caller (#871).
Provenance is proven only when ALL hold:
* both commits are present (a rewritten/force-moved prior head leaves the
object graph and fails closed);
* ``prior_head_sha`` is a strict ancestor of ``synced_head_sha`` (the branch
history is preserved, never replaced);
* ``synced_head_sha`` is a merge commit (two or more parents), i.e. a base
merged in — a plain fast-forward of new direct commits is not a sync;
* ``prior_head_sha`` is an ancestor of the merge's **first** parent, so the
branch mainline (first-parent lineage) still reaches the prior head — a
rebase/force-push that re-authored the branch side fails this.
"""
path = (worktree_path or "").strip()
prior = (prior_head_sha or "").strip()
synced = (synced_head_sha or "").strip()
result: dict = {
"prior_head_sha": prior or None,
"synced_head_sha": synced or None,
"probe_ok": False,
"prior_present": False,
"synced_present": False,
"prior_is_ancestor": False,
"synced_is_merge": False,
"first_parent_reaches_prior": False,
"is_merge_sync": False,
"first_parent_sha": None,
"parent_count": None,
"proof": None,
"reasons": [],
}
if not path or not prior or not synced:
result["reasons"].append(
"merge-sync provenance probe requires a worktree path and both "
"commit SHAs"
)
return result
if prior == synced:
result["reasons"].append(
"prior and synced heads are identical; no branch sync occurred"
)
return result
def _present(sha: str) -> bool:
res = subprocess.run(
["git", "-C", path, "rev-parse", "--verify", "--quiet", f"{sha}^{{commit}}"],
capture_output=True,
text=True,
check=False,
)
return res.returncode == 0
def _is_ancestor(ancestor: str, descendant: str) -> bool | None:
res = subprocess.run(
["git", "-C", path, "merge-base", "--is-ancestor", ancestor, descendant],
capture_output=True,
text=True,
check=False,
)
if res.returncode == 0:
return True
if res.returncode == 1:
return False
return None # failed probe — never a silent "no"
try:
result["prior_present"] = _present(prior)
result["synced_present"] = _present(synced)
except OSError as exc: # git unavailable — fail closed, never assume
result["reasons"].append(f"merge-sync provenance probe could not run: {exc}")
return result
if not result["prior_present"]:
result["reasons"].append(
f"prior head {prior} is not reachable in '{path}'; history may have "
"been rewritten or force-moved"
)
if not result["synced_present"]:
result["reasons"].append(
f"synced head {synced} is not reachable in '{path}'"
)
if not (result["prior_present"] and result["synced_present"]):
return result
ancestor = _is_ancestor(prior, synced)
if ancestor is None:
result["reasons"].append(
"ancestry probe failed; merge-sync provenance unproven"
)
return result
result["prior_is_ancestor"] = bool(ancestor)
if not ancestor:
result["reasons"].append(
f"prior head {prior} is not an ancestor of synced head {synced}; "
"the branch history was not preserved (not a merge-based sync)"
)
return result
parents_res = subprocess.run(
["git", "-C", path, "rev-list", "--parents", "-n", "1", synced],
capture_output=True,
text=True,
check=False,
)
if parents_res.returncode != 0:
result["reasons"].append(
f"could not read parents of {synced}; merge-sync provenance unproven"
)
return result
tokens = (parents_res.stdout or "").split()
# tokens[0] is the commit itself; the rest are its parents.
parents = tokens[1:]
result["parent_count"] = len(parents)
result["synced_is_merge"] = len(parents) >= 2
if not result["synced_is_merge"]:
result["probe_ok"] = True
result["reasons"].append(
f"synced head {synced} has {len(parents)} parent(s); a merge-based "
"branch sync produces a merge commit (two or more parents)"
)
return result
first_parent = parents[0]
result["first_parent_sha"] = first_parent
fp_reaches = _is_ancestor(prior, first_parent) if prior != first_parent else True
if fp_reaches is None:
result["reasons"].append(
"first-parent ancestry probe failed; merge-sync provenance unproven"
)
return result
result["first_parent_reaches_prior"] = bool(fp_reaches)
result["probe_ok"] = True
if not fp_reaches:
result["reasons"].append(
f"merge first parent {first_parent} does not reach prior head "
f"{prior}; the branch mainline was re-authored (not a sanctioned sync)"
)
return result
result["is_merge_sync"] = True
result["proof"] = (
f"synced head {synced} is a merge commit (parents={len(parents)}) whose "
f"first-parent lineage reaches prior head {prior}; base merged into branch"
)
return result
def read_recorded_base( def read_recorded_base(
worktree_path: str, worktree_path: str,
*, *,
-352
View File
@@ -1639,358 +1639,6 @@ class TestSecondRemediationIntegration(unittest.TestCase):
self.assertTrue(ownership_calls) self.assertTrue(ownership_calls)
class TestIssue855ExactPrSelector(unittest.TestCase):
"""#855: exact pr_number pin for reconcile_merged_cleanups (#851 lifecycle)."""
def setUp(self):
self._remotes = patch.dict(
mcp_server.REMOTES,
{
"prgs": {
"host": "gitea.example.com",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
}
},
)
self._remotes.start()
patch("gitea_audit.audit_enabled", return_value=False).start()
self.mock_api = patch("mcp_server.api_request").start()
self.mock_all = patch("mcp_server.api_get_all", return_value=[]).start()
patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
patch(
"mcp_server.merged_cleanup_reconcile.is_head_ancestor_of_ref",
return_value=True,
).start()
patch(
"mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE),
).start()
patch(
"mcp_server._profile_operation_gate",
return_value=[],
).start()
patch(
"mcp_server._collect_branch_ownership_records",
return_value={"records": [], "inventory_error": False},
).start()
patch(
"mcp_server.merged_cleanup_reconcile.discover_reviewer_scratch_worktrees",
return_value=[],
).start()
patch("mcp_server.verify_preflight_purity", return_value=None).start()
patch(
"mcp_server.audit_reconciliation_mode.check_cleanup_execution_allowed",
return_value=(True, []),
).start()
def tearDown(self):
patch.stopall()
def _merged_pr(self, number, branch, sha="c" * 40):
return {
"number": number,
"title": f"PR {number}",
"body": f"Closes #{number - 4}",
"merged": True,
"merged_at": "2026-07-23T12:00:00Z",
"merge_commit_sha": "f" * 40,
"state": "closed",
"head": {"ref": branch, "sha": sha},
"base": {"ref": "master"},
}
def test_exact_pr_848_ignores_newer_852_in_batch_queue(self):
"""pr_number=848 selects only #848 even when #852 is newer/first."""
from mcp_server import gitea_reconcile_merged_cleanups
pr_848 = self._merged_pr(
848, "fix/issue-844-exclude-epic-containers", sha="c3f282ba" + "0" * 32
)
# Closed list would rank #852 first in batch mode; exact pin must ignore it.
closed_batch = [
self._merged_pr(852, "fix/issue-851-cleanup-worktree-before-remote-delete"),
pr_848,
self._merged_pr(849, "fix/issue-849-other"),
self._merged_pr(846, "fix/issue-846-other"),
self._merged_pr(845, "fix/issue-845-other"),
]
batch_fetch_calls = []
def fake_api(method, url, *args, **kwargs):
if method == "GET" and url.rstrip("/").endswith("/pulls/848"):
return dict(pr_848)
if method == "GET" and "/pulls/" in url:
raise AssertionError(f"unexpected PR fetch: {url}")
if method == "GET" and "/branches/" in url:
return {"name": "present"}
return {}
def fake_all(url, auth, limit=None):
batch_fetch_calls.append((url, limit))
if "state=open" in url:
return []
if "state=closed" in url:
# Exact mode must not use the closed batch list.
raise AssertionError(
"exact pr_number mode must not page closed PRs: " + url
)
return []
self.mock_api.side_effect = fake_api
self.mock_all.side_effect = fake_all
patch(
"mcp_server._remote_branch_exists",
return_value=True,
).start()
patch(
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
side_effect=lambda **kwargs: {
"entries": [
{
"pr_number": int(pr["number"]),
"head_branch": (pr.get("head") or {}).get("ref"),
"issue_number": 844,
"remote_branch": {
"safe_to_delete_remote": True,
"head_branch": (pr.get("head") or {}).get("ref"),
},
"local_worktree": {
"safe_to_remove_worktree": True,
"worktree_path": (
"/tmp/branches/fix-issue-844-exclude-epic-containers"
),
},
"planned_execution_order": (
mcp_server.merged_cleanup_reconcile.plan_cleanup_execution_order(
remote_assessment={"safe_to_delete_remote": True},
local_assessment={"safe_to_remove_worktree": True},
)
),
}
for pr in kwargs.get("closed_prs") or []
if pr.get("merged_at") or pr.get("merged")
],
"reviewer_scratch_entries": [],
"merged_pr_count": len(kwargs.get("closed_prs") or []),
},
).start()
res = gitea_reconcile_merged_cleanups(
dry_run=True,
pr_number=848,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(res.get("success"))
self.assertFalse(res.get("performed"))
self.assertEqual(res.get("selection_mode"), "exact_pr")
self.assertEqual(res.get("selected_pr_number"), 848)
entries = res.get("entries") or []
self.assertEqual(len(entries), 1, entries)
self.assertEqual(entries[0].get("pr_number"), 848)
self.assertEqual(
entries[0].get("head_branch"),
"fix/issue-844-exclude-epic-containers",
)
# No other PR appears in plan.
self.assertEqual(list((res.get("planned_execution_orders") or {}).keys()), ["848"])
plan = (res.get("planned_execution_orders") or {}).get("848") or []
actions = [s.get("action") for s in plan]
self.assertEqual(
actions,
[
"remove_local_worktree",
"reassess_branch_ownership",
"delete_remote_branch",
],
)
# Prove we never scanned the multi-PR closed batch.
self.assertFalse(any("state=closed" in (u or "") for u, _ in batch_fetch_calls))
# closed_batch fixture must remain unused (sanity).
self.assertEqual(closed_batch[0]["number"], 852)
def test_exact_pr_execute_only_mutates_selected_pr(self):
"""Execute with pr_number must never touch #845/#846/#849/#852."""
from mcp_server import gitea_reconcile_merged_cleanups
pr_848 = self._merged_pr(848, "fix/issue-844-exclude-epic-containers")
worktree_path = "/tmp/branches/fix-issue-844-exclude-epic-containers"
remove_calls = []
delete_api_calls = []
ownership_branches = []
def fake_api(method, url, *args, **kwargs):
if method == "GET" and url.rstrip("/").endswith("/pulls/848"):
return dict(pr_848)
if method == "DELETE":
delete_api_calls.append(url)
# Forbid foreign PR branch deletion by URL content.
for forbidden in ("845", "846", "849", "852"):
self.assertNotIn(forbidden, url)
return {}
def fake_remove(project_root, branch, worktree_path=None):
remove_calls.append({"branch": branch, "worktree_path": worktree_path})
return {
"success": True,
"performed": True,
"message": f"removed {worktree_path}",
"worktree_path": worktree_path,
}
def fake_collect(**kwargs):
ownership_branches.append(kwargs.get("branch"))
return {"records": [], "inventory_error": False}
def fake_probe(h, o, r, auth, br):
return guard.classify_branch_readback_http_status(
404, not_found_scope=guard.NOT_FOUND_SCOPE_BRANCH
)
self.mock_api.side_effect = fake_api
self.mock_all.side_effect = lambda url, auth, limit=None: []
patch("mcp_server._remote_branch_exists", return_value=True).start()
patch(
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
return_value={
"entries": [
{
"pr_number": 848,
"head_branch": "fix/issue-844-exclude-epic-containers",
"remote_branch": {"safe_to_delete_remote": True},
"local_worktree": {
"safe_to_remove_worktree": True,
"worktree_path": worktree_path,
},
"planned_execution_order": [
{"action": "remove_local_worktree", "phase": 1},
{"action": "reassess_branch_ownership", "phase": 2},
{"action": "delete_remote_branch", "phase": 3},
],
}
],
"reviewer_scratch_entries": [
# Foreign scratch must be filtered before report execute loop;
# if present here it would still be a test failure if acted on.
],
"merged_pr_count": 1,
},
).start()
patch(
"mcp_server.merged_cleanup_reconcile.remove_local_worktree",
side_effect=fake_remove,
).start()
patch(
"mcp_server._collect_branch_ownership_records",
side_effect=fake_collect,
).start()
patch("mcp_server._probe_remote_branch", side_effect=fake_probe).start()
res = gitea_reconcile_merged_cleanups(
dry_run=False,
execute_confirmed=True,
pr_number=848,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
self.assertTrue(res.get("performed") or res.get("executed"))
self.assertEqual(res.get("selection_mode"), "exact_pr")
self.assertEqual(res.get("selected_pr_number"), 848)
actions = res.get("actions") or []
pr_numbers_touched = {
a.get("pr_number") for a in actions if a.get("pr_number") is not None
}
self.assertTrue(pr_numbers_touched.issubset({None, 848}) or not pr_numbers_touched)
removes = [a for a in actions if a.get("action") == "remove_local_worktree"]
deletes = [a for a in actions if a.get("action") == "delete_remote_branch"]
self.assertEqual(len(removes), 1)
self.assertEqual(remove_calls[0]["branch"], "fix/issue-844-exclude-epic-containers")
self.assertEqual(len(deletes), 1)
self.assertTrue(deletes[0].get("success"))
self.assertTrue(deletes[0].get("after_worktree_removal"))
self.assertEqual(len(delete_api_calls), 1)
self.assertEqual(
ownership_branches, ["fix/issue-844-exclude-epic-containers"]
)
def test_exact_pr_unknown_fails_closed_without_mutation(self):
from mcp_server import gitea_reconcile_merged_cleanups
def fake_api(method, url, *args, **kwargs):
if method == "GET" and "/pulls/99999" in url:
raise RuntimeError("HTTP 404 Not Found")
raise AssertionError(f"unexpected API call {method} {url}")
self.mock_api.side_effect = fake_api
res = gitea_reconcile_merged_cleanups(
dry_run=True,
pr_number=99999,
remote="prgs",
)
self.assertFalse(res.get("success"))
self.assertFalse(res.get("performed"))
self.assertEqual(res.get("blocker_kind"), "pr_unresolvable")
self.assertIn("99999", " ".join(res.get("reasons") or []))
def test_exact_pr_not_merged_fails_closed(self):
from mcp_server import gitea_reconcile_merged_cleanups
def fake_api(method, url, *args, **kwargs):
if method == "GET" and url.rstrip("/").endswith("/pulls/900"):
return {
"number": 900,
"merged": False,
"merged_at": None,
"state": "open",
"head": {"ref": "feat/x", "sha": "a" * 40},
}
raise AssertionError(f"unexpected {method} {url}")
self.mock_api.side_effect = fake_api
res = gitea_reconcile_merged_cleanups(
dry_run=False,
execute_confirmed=True,
pr_number=900,
remote="prgs",
)
self.assertFalse(res.get("success"))
self.assertFalse(res.get("performed"))
self.assertEqual(res.get("blocker_kind"), "pr_not_merged")
def test_exact_pr_invalid_number_fails_closed(self):
from mcp_server import gitea_reconcile_merged_cleanups
res = gitea_reconcile_merged_cleanups(
dry_run=True,
pr_number=0,
remote="prgs",
)
self.assertFalse(res.get("success"))
self.assertEqual(res.get("blocker_kind"), "invalid_pr_number")
self.mock_api.assert_not_called()
def test_batch_mode_still_works_without_pr_number(self):
"""Unfiltered batch path remains backward compatible."""
from mcp_server import gitea_reconcile_merged_cleanups
self.mock_all.side_effect = lambda url, auth, limit=None: []
self.mock_api.side_effect = lambda *a, **k: {}
patch(
"mcp_server.merged_cleanup_reconcile.build_reconciliation_report",
return_value={
"entries": [],
"reviewer_scratch_entries": [],
"merged_pr_count": 0,
},
).start()
res = gitea_reconcile_merged_cleanups(dry_run=True, remote="prgs", limit=10)
self.assertTrue(res.get("success"))
self.assertEqual(res.get("selection_mode"), "batch")
self.assertIsNone(res.get("selected_pr_number"))
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
@@ -1,244 +0,0 @@
"""#855 AC4: an expired reviewer lease must not indefinitely protect an
already-merged branch when no live claimant exists.
Two layers are covered:
* ``branch_cleanup_guard.assess_expired_reviewer_lease_reclaim`` the pure,
fail-closed reclaim decision. Every condition must be provably satisfied or
the lease keeps protecting the branch.
* ``gitea_mcp_server._collect_branch_ownership_records`` the wiring that
supplies authoritative evidence (PR merged state, owner-process liveness,
competing ownership) to that decision, and flips an expired reviewer lease
to reclaimable only under the full policy.
All inputs are fabricated; no real repository, lease, or credential is used.
"""
import importlib
import unittest
from unittest.mock import patch
import branch_cleanup_guard
mcp_server = importlib.import_module("gitea_mcp_server")
FAKE_AUTH = "token fake"
REMOTE = "prgs"
ORG = "Scaled-Tech-Consulting"
REPO = "Gitea-Tools"
HOST = "gitea.prgs.cc"
BRANCH = "feat/issue-638-webui-app-shell-phase1"
PR_NUMBER = 818
class TestAssessExpiredReviewerLeaseReclaim(unittest.TestCase):
"""Pure fail-closed reclaim decision (#855 AC4)."""
def _call(self, **overrides):
base = dict(
role="reviewer",
status="expired",
pr_merged=True,
owner_pid_alive=False,
competing_active_claimant=False,
)
base.update(overrides)
return branch_cleanup_guard.assess_expired_reviewer_lease_reclaim(**base)
def test_full_policy_satisfied_allows_reclaim(self):
out = self._call()
self.assertTrue(out["reclaim_allowed"])
self.assertEqual(out["reasons"], [])
self.assertEqual(out["decision"], "reclaim_expired_reviewer_lease")
def test_stale_dead_process_reviewer_also_reclaimable(self):
out = self._call(status="stale_dead_process")
self.assertTrue(out["reclaim_allowed"])
def test_non_reviewer_role_never_reclaims(self):
for role in ("author", "merger", "controller", "reconciler", "unknown"):
with self.subTest(role=role):
out = self._call(role=role)
self.assertFalse(out["reclaim_allowed"])
self.assertTrue(out["reasons"])
self.assertEqual(out["decision"], "keep_protecting")
def test_active_status_never_reclaims(self):
out = self._call(status="active")
self.assertFalse(out["reclaim_allowed"])
def test_pr_not_merged_blocks_reclaim(self):
out = self._call(pr_merged=False)
self.assertFalse(out["reclaim_allowed"])
def test_pr_merged_unknown_fails_closed(self):
out = self._call(pr_merged=None)
self.assertFalse(out["reclaim_allowed"])
def test_owner_process_alive_blocks_reclaim(self):
out = self._call(owner_pid_alive=True)
self.assertFalse(out["reclaim_allowed"])
def test_owner_liveness_unknown_fails_closed(self):
out = self._call(owner_pid_alive=None)
self.assertFalse(out["reclaim_allowed"])
def test_competing_active_claimant_blocks_reclaim(self):
out = self._call(competing_active_claimant=True)
self.assertFalse(out["reclaim_allowed"])
def test_competing_claimant_unknown_fails_closed(self):
out = self._call(competing_active_claimant=None)
self.assertFalse(out["reclaim_allowed"])
def test_reasons_never_leak_secrets(self):
out = self._call(role="author")
blob = " ".join(out["reasons"]).lower()
self.assertNotIn("token", blob)
self.assertNotIn("password", blob)
class _FakeLease(dict):
pass
class TestCollectorExpiredReviewerReclaimWiring(unittest.TestCase):
"""`_collect_branch_ownership_records` supplies authoritative evidence and
flips an expired reviewer lease to reclaimable only under the full policy."""
def _run(
self,
*,
lease_role="reviewer",
lease_freshness="stale_dead_process",
owner_pid_alive=False,
pr_merged=True,
extra_leases=None,
worktree_on_branch=False,
):
lease = _FakeLease(
role=lease_role,
work_kind="pr",
work_number=PR_NUMBER,
branch=BRANCH,
status="active",
owner_pid=999999,
remote=REMOTE,
org=ORG,
repo=REPO,
host=HOST,
freshness={
"freshness": lease_freshness,
"owner_pid": 999999,
"owner_pid_alive": owner_pid_alive,
"expired_by_time": lease_freshness == "expired",
},
)
leases = [lease] + list(extra_leases or [])
pr_payload = {
"number": PR_NUMBER,
"merged": pr_merged,
"merged_at": "2026-07-23T00:00:00Z" if pr_merged else None,
"head": {"ref": BRANCH},
}
def fake_api_request(method, url, *a, **k):
if method == "GET" and f"/pulls/{PR_NUMBER}" in url:
return pr_payload
raise AssertionError(f"unexpected api_request {method} {url}")
wt_entries = []
if worktree_on_branch:
wt_entries = [{"branch": BRANCH, "path": f"/x/branches/{BRANCH}"}]
with patch.object(
mcp_server.lease_lifecycle,
"list_active_leases",
return_value={"leases": leases},
), patch.object(
mcp_server.control_plane_db, "get_db", return_value=object(), create=True
), patch.object(
mcp_server.issue_lock_store, "iter_lock_files", return_value=[]
), patch.object(
mcp_server.worktree_cleanup_audit,
"list_worktrees",
return_value=wt_entries,
), patch.object(
mcp_server, "api_get_all", return_value=[]
), patch.object(
mcp_server, "api_request", side_effect=fake_api_request
):
return mcp_server._collect_branch_ownership_records(
remote=REMOTE,
host=HOST,
org=ORG,
repo=REPO,
branch=BRANCH,
pr_number=PR_NUMBER,
project_root="/x",
auth=FAKE_AUTH,
base_api="https://gitea.prgs.cc/api/v1/repos/x/y",
)
def _reviewer_records(self, bundle):
return [
rec
for rec in bundle["records"]
if rec.get("category")
== branch_cleanup_guard.OWNERSHIP_CATEGORY_REVIEWER_LEASE
]
def test_merged_dead_uncontested_reviewer_lease_is_reclaimable(self):
bundle = self._run()
self.assertFalse(bundle["inventory_error"])
recs = self._reviewer_records(bundle)
self.assertEqual(len(recs), 1)
self.assertTrue(recs[0]["reclaim_allowed"])
# And the guard consequently does not block deletion on it.
ownership = branch_cleanup_guard.assess_active_branch_ownership(
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
records=bundle["records"],
)
self.assertFalse(ownership["block"])
def test_unmerged_pr_keeps_reviewer_lease_protective(self):
bundle = self._run(pr_merged=False)
recs = self._reviewer_records(bundle)
self.assertEqual(len(recs), 1)
self.assertFalse(recs[0]["reclaim_allowed"])
ownership = branch_cleanup_guard.assess_active_branch_ownership(
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
records=bundle["records"],
)
self.assertTrue(ownership["block"])
def test_owner_process_alive_keeps_reviewer_lease_protective(self):
bundle = self._run(owner_pid_alive=True, lease_freshness="expired")
recs = self._reviewer_records(bundle)
self.assertFalse(recs[0]["reclaim_allowed"])
def test_competing_worktree_binding_keeps_reviewer_lease_protective(self):
bundle = self._run(worktree_on_branch=True)
recs = self._reviewer_records(bundle)
self.assertFalse(recs[0]["reclaim_allowed"])
ownership = branch_cleanup_guard.assess_active_branch_ownership(
remote=REMOTE, org=ORG, repo=REPO, branch=BRANCH, host=HOST,
records=bundle["records"],
)
self.assertTrue(ownership["block"])
def test_expired_author_lease_never_reclaimed_by_reviewer_policy(self):
bundle = self._run(lease_role="author")
author_recs = [
rec
for rec in bundle["records"]
if rec.get("category")
== branch_cleanup_guard.OWNERSHIP_CATEGORY_AUTHOR_LEASE
]
self.assertEqual(len(author_recs), 1)
self.assertFalse(author_recs[0]["reclaim_allowed"])
if __name__ == "__main__":
unittest.main()
@@ -1,630 +0,0 @@
"""Durable linked-issue lock head refresh + merge-sync dead-session recovery (#871).
``gitea_update_pr_branch_by_merge`` advances a PR's *remote* head but historically
never advanced the linked durable issue lock's recorded head. After the owning
session died the drifted lock became unrecoverable and no further synchronization
was possible (PR #866 / issue #855).
Two halves are covered:
* the write-side refresh (``issue_lock_store.assess/apply_durable_lock_head_refresh``)
that records the new synced head under compare-and-swap with read-after-write; and
* the read-side recovery relation (``issue_lock_recovery`` +
``issue_lock_worktree.read_merge_sync_provenance``) that lets a dead-session lock
whose recorded head is a merge-sync *ancestor* of the live PR head be recovered
and nothing else.
"""
from __future__ import annotations
import os
import subprocess
import sys
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import issue_lock_recovery # noqa: E402
import issue_lock_store # noqa: E402
import issue_lock_worktree # noqa: E402
ISSUE = 8710
PR_NUMBER = 8711
BRANCH = f"fix/issue-{ISSUE}-durable-lock-head-refresh"
IDENTITY = "example-user"
PROFILE = "example-author"
OLD = "a" * 40
NEW1 = "b" * 40
NEW2 = "c" * 40
BASE = "d" * 40
REMOTE = "prgs"
ORG = "ExampleOrg"
REPO = "ExampleRepo"
def dead_pid() -> int:
proc = subprocess.Popen([sys.executable, "-c", "pass"])
proc.wait()
return proc.pid
def future_ts(hours: int = 4) -> str:
return (
(datetime.now(timezone.utc) + timedelta(hours=hours))
.isoformat()
.replace("+00:00", "Z")
)
def _git(cwd, *args):
return subprocess.run(
["git", "-C", cwd, *args],
capture_output=True,
text=True,
check=True,
)
def _rev(cwd, ref="HEAD") -> str:
return _git(cwd, "rev-parse", ref).stdout.strip()
def build_merge_sync_repo(tmp: str) -> dict:
"""Build a repo where a feature branch was synced by merging master in.
Returns a dict with the prior (branch) head, the synced merge-commit head,
the master tip, plus a rebase-style linear descendant and an unrelated head.
"""
_git(tmp, "init", "-q", "-b", "master")
_git(tmp, "config", "user.email", "[email protected]")
_git(tmp, "config", "user.name", "T")
Path(tmp, "base.txt").write_text("base\n")
_git(tmp, "add", "-A")
_git(tmp, "commit", "-q", "-m", "root")
# Feature branch cut from root, one commit — this is the PRIOR/recorded head.
_git(tmp, "checkout", "-q", "-b", BRANCH)
Path(tmp, "feature.txt").write_text("feature\n")
_git(tmp, "add", "-A")
_git(tmp, "commit", "-q", "-m", "feature work")
prior = _rev(tmp)
# Master advances (the base the sync will merge in).
_git(tmp, "checkout", "-q", "master")
Path(tmp, "base.txt").write_text("base\nmore\n")
_git(tmp, "add", "-A")
_git(tmp, "commit", "-q", "-m", "master advance")
master_tip = _rev(tmp)
# Sync: merge master INTO the feature branch → merge commit, first parent = prior.
_git(tmp, "checkout", "-q", BRANCH)
_git(tmp, "merge", "-q", "--no-ff", "-m", "Merge master into feature", "master")
synced = _rev(tmp)
# A plain linear descendant of prior (NOT a merge) — a rebase/extra-commit shape.
_git(tmp, "checkout", "-q", "-b", "linear-branch", prior)
Path(tmp, "extra.txt").write_text("extra\n")
_git(tmp, "add", "-A")
_git(tmp, "commit", "-q", "-m", "extra linear commit")
linear = _rev(tmp)
# An unrelated root (force-push / rewritten history shape).
unrelated_dir = tempfile.mkdtemp()
_git(unrelated_dir, "init", "-q", "-b", "x")
_git(unrelated_dir, "config", "user.email", "[email protected]")
_git(unrelated_dir, "config", "user.name", "T")
Path(unrelated_dir, "z.txt").write_text("z\n")
_git(unrelated_dir, "add", "-A")
_git(unrelated_dir, "commit", "-q", "-m", "unrelated")
unrelated = _rev(unrelated_dir)
# Leave the worktree checked out on the feature branch at the PRIOR head, as
# a dead author session that never advanced would have left it.
_git(tmp, "checkout", "-q", BRANCH)
_git(tmp, "reset", "-q", "--hard", prior)
return {
"prior": prior,
"master_tip": master_tip,
"synced": synced,
"linear": linear,
"unrelated": unrelated,
}
# ─────────────────────────── write-side refresh ───────────────────────────
class TestDurableLockHeadRefresh(unittest.TestCase):
def setUp(self):
self.lock_dir = tempfile.mkdtemp()
self.wt = tempfile.mkdtemp()
lock_data = {
"issue_number": ISSUE,
"branch_name": BRANCH,
"worktree_path": self.wt,
"remote": REMOTE,
"org": ORG,
"repo": REPO,
"claimant": {"username": IDENTITY, "profile": PROFILE},
"work_lease": {
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
"issue_number": ISSUE,
"branch": BRANCH,
"worktree_path": self.wt,
"claimant": {"username": IDENTITY, "profile": PROFILE},
"expires_at": future_ts(),
},
}
issue_lock_store.bind_session_lock(lock_data, lock_dir=self.lock_dir)
def _apply(self, **over):
kw = dict(
remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE,
branch_name=BRANCH, worktree_path=self.wt, pr_number=PR_NUMBER,
identity=IDENTITY, profile=PROFILE, current_pid=os.getpid(),
expected_old_head=OLD, new_head=NEW1, synced_at=future_ts(0),
base_head=BASE, lock_dir=self.lock_dir,
)
kw.update(over)
return issue_lock_store.apply_durable_lock_head_refresh(**kw)
def _load(self):
return issue_lock_store.load_issue_lock(
remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE,
lock_dir=self.lock_dir,
)
def test_first_sync_updates_recorded_head(self):
"""AC1: first base sync writes the resulting head to the durable lock."""
res = self._apply()
self.assertTrue(res["refreshed"], res["reasons"])
self.assertTrue(res["read_after_write_ok"])
self.assertEqual(self._load().get("synced_pr_head"), NEW1)
def test_second_sync_after_master_advance(self):
"""AC2: a later master advance permits a second sanctioned sync."""
self.assertTrue(self._apply()["refreshed"])
res2 = self._apply(expected_old_head=NEW1, new_head=NEW2)
self.assertTrue(res2["refreshed"], res2["reasons"])
self.assertEqual(self._load().get("synced_pr_head"), NEW2)
history = self._load().get("branch_sync_history")
self.assertEqual(len(history), 2)
self.assertEqual(history[0]["last_synced_pr_head"], NEW1)
self.assertEqual(history[1]["prior_pr_head"], NEW1)
def test_cas_detects_concurrent_head_change(self):
"""AC6: CAS refuses when the recorded synced head is not the old head."""
self.assertTrue(self._apply()["refreshed"]) # recorded head now NEW1
# A second sync claiming the old head is still OLD must fail closed.
res = self._apply(expected_old_head=OLD, new_head=NEW2)
self.assertFalse(res["refreshed"])
self.assertTrue(any("CAS" in r or "concurrent" in r for r in res["reasons"]))
self.assertEqual(self._load().get("synced_pr_head"), NEW1)
def test_wrong_issue_fails_closed(self):
res = self._apply(issue_number=999999)
self.assertFalse(res["refreshed"])
def test_wrong_branch_fails_closed(self):
res = self._apply(branch_name="fix/issue-8710-wrong")
self.assertFalse(res["refreshed"])
def test_wrong_repo_fails_closed(self):
res = self._apply(repo="OtherRepo")
self.assertFalse(res["refreshed"])
def test_wrong_identity_fails_closed(self):
res = self._apply(identity="intruder")
self.assertFalse(res["refreshed"])
def test_wrong_profile_fails_closed(self):
res = self._apply(profile="prgs-reviewer")
self.assertFalse(res["refreshed"])
def test_foreign_session_fails_closed(self):
"""A refresh is not a recovery: the current process must own the lock."""
path = issue_lock_store.lock_file_path(
remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE,
lock_dir=self.lock_dir,
)
rec = issue_lock_store.read_lock_file(path)
rec["session_pid"] = dead_pid()
rec["pid"] = rec["session_pid"]
issue_lock_store.save_lock_file(path, rec)
res = self._apply()
self.assertFalse(res["refreshed"])
self.assertTrue(any("current session" in r or "live owner" in r for r in res["reasons"]))
def test_new_equals_old_fails_closed(self):
res = self._apply(expected_old_head=OLD, new_head=OLD)
self.assertFalse(res["refreshed"])
def test_non_full_sha_fails_closed(self):
self.assertFalse(self._apply(new_head="deadbeef")["refreshed"])
self.assertFalse(self._apply(expected_old_head="xyz")["refreshed"])
def test_no_lock_fails_closed(self):
assessment = issue_lock_store.assess_durable_lock_head_refresh(
None, remote=REMOTE, org=ORG, repo=REPO, issue_number=ISSUE,
branch_name=BRANCH, worktree_path=self.wt, pr_number=PR_NUMBER,
identity=IDENTITY, profile=PROFILE, current_pid=os.getpid(),
expected_old_head=OLD, new_head=NEW1,
)
self.assertFalse(assessment["allowed"])
# ─────────────────────── merge-sync provenance (real git) ───────────────────
class TestMergeSyncProvenanceObservation(unittest.TestCase):
def setUp(self):
self.tmp = tempfile.mkdtemp()
self.shas = build_merge_sync_repo(self.tmp)
def test_merge_sync_is_recognized(self):
obs = issue_lock_worktree.read_merge_sync_provenance(
self.tmp, prior_head_sha=self.shas["prior"],
synced_head_sha=self.shas["synced"],
)
self.assertTrue(obs["is_merge_sync"], obs["reasons"])
self.assertTrue(obs["prior_is_ancestor"])
self.assertTrue(obs["synced_is_merge"])
self.assertTrue(obs["first_parent_reaches_prior"])
def test_linear_descendant_is_not_a_merge_sync(self):
"""A plain non-merge descendant (rebase/extra commit) is not a sync."""
obs = issue_lock_worktree.read_merge_sync_provenance(
self.tmp, prior_head_sha=self.shas["prior"],
synced_head_sha=self.shas["linear"],
)
self.assertTrue(obs["probe_ok"])
self.assertFalse(obs["is_merge_sync"])
self.assertFalse(obs["synced_is_merge"])
def test_unrelated_history_fails_closed(self):
"""A rewritten/force-pushed head where prior is unreachable fails closed."""
obs = issue_lock_worktree.read_merge_sync_provenance(
self.tmp, prior_head_sha=self.shas["prior"],
synced_head_sha=self.shas["unrelated"],
)
self.assertFalse(obs["is_merge_sync"])
def test_missing_args_fail_closed(self):
obs = issue_lock_worktree.read_merge_sync_provenance(
self.tmp, prior_head_sha=None, synced_head_sha=self.shas["synced"],
)
self.assertFalse(obs["is_merge_sync"])
# ──────────────────── merge-sync dead-session recovery ──────────────────────
def make_dead_lock(worktree, **over):
pid = dead_pid()
lock = {
"issue_number": ISSUE,
"branch_name": BRANCH,
"worktree_path": worktree,
"remote": REMOTE,
"org": ORG,
"repo": REPO,
"session_pid": pid,
"pid": pid,
"claimant": {"username": IDENTITY, "profile": PROFILE},
"work_lease": {
"operation_type": issue_lock_store.AUTHOR_ISSUE_WORK_LEASE,
"issue_number": ISSUE,
"branch": BRANCH,
"worktree_path": worktree,
"claimant": {"username": IDENTITY, "profile": PROFILE},
"expires_at": future_ts(),
},
}
lock.update(over)
return lock
def sync_prov(prior, synced, **over):
d = {
"prior_head_sha": prior,
"synced_head_sha": synced,
"probe_ok": True,
"prior_present": True,
"synced_present": True,
"prior_is_ancestor": True,
"synced_is_merge": True,
"first_parent_reaches_prior": True,
"is_merge_sync": True,
"first_parent_sha": prior,
"parent_count": 2,
"proof": f"{synced} merged base into branch above {prior}",
"reasons": [],
}
d.update(over)
return d
class TestMergeSyncRecovery(unittest.TestCase):
def setUp(self):
self.tmp = tempfile.mkdtemp()
self.shas = build_merge_sync_repo(self.tmp)
self.prior = self.shas["prior"]
self.synced = self.shas["synced"]
def _assess(self, **over):
lock = over.pop("_lock", None) or make_dead_lock(self.tmp)
kw = dict(
issue_number=ISSUE, branch_name=BRANCH, worktree_path=self.tmp,
remote=REMOTE, org=ORG, repo=REPO, identity=IDENTITY, profile=PROFILE,
current_branch=BRANCH, porcelain_status="",
head_sha=self.prior, remote_head_sha=self.synced,
pr_head_sha=self.synced, pr_number=PR_NUMBER,
competing_live_locks=[], candidate_branches=[BRANCH],
current_pid=os.getpid(),
remote_branch_exists=True,
sync_provenance=sync_prov(self.prior, self.synced),
)
kw.update(over)
return issue_lock_recovery.assess_dead_session_lock_recovery(lock, **kw)
def test_merge_sync_drift_is_recoverable(self):
"""AC3/AC4: dead session, recorded head is a merge-sync ancestor of PR head."""
res = self._assess()
self.assertEqual(res["outcome"], issue_lock_recovery.RECOVERY_SANCTIONED, res["reasons"])
self.assertEqual(
res["evidence"]["head_relation"],
issue_lock_recovery.HEAD_RELATION_REMOTE_MERGE_SYNCED,
)
self.assertEqual(res["evidence"]["accepted_head"], self.synced)
def test_missing_provenance_fails_closed(self):
"""No server-derived provenance → cannot accept a remote ahead of local."""
res = self._assess(sync_provenance=None)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_non_ancestor_recorded_head_fails_closed(self):
"""AC7: provenance that does not prove ancestry is rejected."""
res = self._assess(
sync_provenance=sync_prov(
self.prior, self.synced, prior_is_ancestor=False, is_merge_sync=False,
reasons=["prior head is not an ancestor"],
)
)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_force_pushed_history_fails_closed(self):
"""AC8: a rewritten head (not a merge sync) stays protected."""
res = self._assess(
sync_provenance=sync_prov(
self.prior, self.synced, is_merge_sync=False, synced_is_merge=False,
reasons=["not a merge-based sync"],
)
)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_provenance_for_other_commits_fails_closed(self):
"""Provenance whose endpoints differ from the heads under assessment is rejected."""
res = self._assess(
sync_provenance=sync_prov("f" * 40, self.synced),
)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_dirty_worktree_fails_closed(self):
"""AC11: dirty worktrees remain protected."""
res = self._assess(porcelain_status=" M feature.txt\n")
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_live_owner_fails_closed(self):
"""AC10: a live recorded owner is not a dead-session recovery."""
lock = make_dead_lock(self.tmp, session_pid=os.getpid(), pid=os.getpid())
res = self._assess(_lock=lock)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_competing_claimant_fails_closed(self):
"""AC13: a competing live lock blocks recovery."""
res = self._assess(
competing_live_locks=[{
"issue_number": ISSUE, "branch_name": BRANCH,
"worktree_path": "/some/other/wt", "pid": os.getpid(),
}]
)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_wrong_branch_fails_closed(self):
"""AC9: worktree on a different branch fails closed."""
res = self._assess(current_branch="fix/issue-8710-other")
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_wrong_identity_fails_closed(self):
res = self._assess(identity="intruder")
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_pr_head_mismatch_fails_closed(self):
"""The open PR must sit at the synced remote head."""
res = self._assess(pr_head_sha="e" * 40)
self.assertEqual(res["outcome"], issue_lock_recovery.REFUSED)
def test_owning_pr_evidence_for_merge_sync(self):
res = self._assess()
ev = issue_lock_recovery.owning_pr_recovery_evidence(res)
self.assertIsNotNone(ev)
self.assertEqual(ev["pr_number"], PR_NUMBER)
self.assertEqual(ev["head_sha"], self.synced)
self.assertEqual(
ev["head_relation"],
issue_lock_recovery.HEAD_RELATION_REMOTE_MERGE_SYNCED,
)
def test_recovered_owning_pr_from_persisted_record(self):
res = self._assess()
record = issue_lock_recovery.build_recovery_record(res, recovered_at=future_ts(0))
lock = {"issue_number": ISSUE, "branch_name": BRANCH,
"dead_session_recovery": record}
rebuilt = issue_lock_recovery.recovered_owning_pr_from_lock(lock)
self.assertIsNotNone(rebuilt)
self.assertEqual(rebuilt["head_sha"], self.synced)
self.assertEqual(
rebuilt["head_relation"],
issue_lock_recovery.HEAD_RELATION_REMOTE_MERGE_SYNCED,
)
class TestExistingRelationsUnchanged(unittest.TestCase):
"""AC14/AC15: equal-head recovery still works; merge-sync did not weaken it."""
def setUp(self):
self.tmp = tempfile.mkdtemp()
self.shas = build_merge_sync_repo(self.tmp)
def test_equal_head_recovery_still_sanctioned(self):
# Worktree at prior head; remote also at prior head → the #753 equal case.
prior = self.shas["prior"]
lock = make_dead_lock(self.tmp)
res = issue_lock_recovery.assess_dead_session_lock_recovery(
lock, issue_number=ISSUE, branch_name=BRANCH, worktree_path=self.tmp,
remote=REMOTE, org=ORG, repo=REPO, identity=IDENTITY, profile=PROFILE,
current_branch=BRANCH, porcelain_status="",
head_sha=prior, remote_head_sha=prior,
pr_head_sha=prior, pr_number=PR_NUMBER,
competing_live_locks=[], candidate_branches=[BRANCH],
current_pid=os.getpid(), remote_branch_exists=True,
)
self.assertEqual(res["outcome"], issue_lock_recovery.RECOVERY_SANCTIONED, res["reasons"])
self.assertEqual(
res["evidence"]["head_relation"], issue_lock_recovery.HEAD_RELATION_EQUAL,
)
class TestUpdatePrWrapperPartialFailure(unittest.TestCase):
"""AC5/AC16: the tool advances the remote head then refreshes the durable lock.
When the durable refresh fails after the remote advance, the tool must report a
partial lifecycle failure and NOT a fully successful synchronization. Exact PR-
head / base-head pinning is preserved (delegated to the real preflight, stubbed
here only to isolate the post-update lifecycle branch).
"""
def setUp(self):
import gitea_mcp_server as gms # noqa: E402
self.gms = gms
self._orig = {}
def _patch(name, value):
self._orig[name] = getattr(gms, name)
setattr(gms, name, value)
_patch("get_profile", lambda *a, **k: {
"allowed_operations": ["gitea.branch.push"],
"forbidden_operations": [],
"profile_name": "prgs-author",
})
_patch("_role_kind", lambda *a, **k: "author")
_patch("_profile_operation_gate", lambda *a, **k: None)
_patch("_permission_block_report", lambda *a, **k: {})
_patch("_resolve", lambda *a, **k: ("gitea.prgs.cc", ORG, REPO))
_patch("_verify_role_mutation_workspace", lambda *a, **k: None)
_patch("_get_workspace_porcelain", lambda *a, **k: "")
_patch("_canonical_local_git_root", lambda *a, **k: "/x")
_patch("_master_parity_block", lambda *a, **k: None)
_patch("_auth", lambda *a, **k: {"token": "x"})
_patch("repo_api_url", lambda *a, **k: "http://api")
_patch("_redact", lambda s: s)
_patch("_work_lease_claimant", lambda *a, **k: {
"username": IDENTITY, "profile": PROFILE,
})
_patch("_prove_author_ownership_for_pr", lambda *a, **k: {
"has_author_lock": True, "matched_issue": ISSUE,
"matched_via": "branch", "linked_issues": [ISSUE],
"recovered_owning_pr": None, "reasons": [],
})
# Real preflight is unit-tested elsewhere; stub it to isolate the
# post-update durable-lock lifecycle branch under test.
orig_pf = gms.pr_sync_status.assess_update_pr_branch_preflight
self._orig_pf = orig_pf
gms.pr_sync_status.assess_update_pr_branch_preflight = (
lambda *a, **k: {"mutation_allowed": True, "reasons": [], "performed": False}
)
# Sequence the two GET /pulls calls: OLD before update, NEW after.
self._pull_calls = {"n": 0}
def fake_api_request(method, url, auth, *a, **k):
m = method.upper()
if m == "GET" and url.endswith(f"/pulls/{PR_NUMBER}"):
self._pull_calls["n"] += 1
head = OLD if self._pull_calls["n"] == 1 else NEW1
return {
"state": "open",
"head": {"sha": head, "ref": BRANCH},
"base": {"sha": BASE, "ref": "master"},
"mergeable": True, "title": "t", "body": "b",
}
if m == "GET" and "/branches/" in url:
return {"commit": {"id": BASE}}
if m == "POST" and "/update" in url:
return {}
return {}
_patch("api_request", fake_api_request)
def tearDown(self):
for name, value in self._orig.items():
setattr(self.gms, name, value)
self.gms.pr_sync_status.assess_update_pr_branch_preflight = self._orig_pf
def _run(self):
return self.gms.gitea_update_pr_branch_by_merge(
pr_number=PR_NUMBER,
expected_pr_head_sha=OLD,
expected_base_head_sha=BASE,
remote=REMOTE,
worktree_path="/tmp/branches/wt-871",
)
def test_partial_failure_when_refresh_fails(self):
self._orig["apply_durable_lock_head_refresh"] = (
self.gms.issue_lock_store.apply_durable_lock_head_refresh
)
self.gms.issue_lock_store.apply_durable_lock_head_refresh = (
lambda **k: {"refreshed": False, "reasons": ["forced refresh failure"]}
)
try:
res = self._run()
finally:
self.gms.issue_lock_store.apply_durable_lock_head_refresh = (
self._orig["apply_durable_lock_head_refresh"]
)
self.assertTrue(res["performed"])
self.assertEqual(res["new_pr_head_sha"], NEW1)
self.assertFalse(res["success"])
self.assertTrue(res["partial_lifecycle_failure"])
self.assertFalse(res["durable_lock_refreshed"])
def test_full_success_when_refresh_succeeds(self):
self._orig["apply_durable_lock_head_refresh"] = (
self.gms.issue_lock_store.apply_durable_lock_head_refresh
)
self.gms.issue_lock_store.apply_durable_lock_head_refresh = (
lambda **k: {"refreshed": True, "read_after_write_ok": True,
"new_head": NEW1, "reasons": ["ok"]}
)
try:
res = self._run()
finally:
self.gms.issue_lock_store.apply_durable_lock_head_refresh = (
self._orig["apply_durable_lock_head_refresh"]
)
self.assertTrue(res["success"])
self.assertTrue(res["performed"])
self.assertTrue(res["durable_lock_refreshed"])
self.assertTrue(res["fully_synchronized"])
self.assertEqual(res["new_pr_head_sha"], NEW1)
if __name__ == "__main__":
unittest.main()
-107
View File
@@ -1,107 +0,0 @@
"""Documentation acceptance for the MCP restart governance ADR (#656).
Enforces issue #656 acceptance criteria:
* AC1 policy document exists with an authorization matrix and the recorded
v1 decision (controller approval + automated safety gates).
* AC2 restart is stated as a last resort with enumerated narrower recoveries.
* AC3 a unilateral LLM full restart with affected sessions is forbidden.
* AC4 break-glass conditions are listed.
* AC5 the ADR is linked to #655, #652, #653, #630, #642, and is cross-linked
from the safety model and the web-console deployment boundary docs.
"""
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
ADR = REPO_ROOT / "docs" / "architecture" / "mcp-restart-governance.md"
ADR_BASENAME = "mcp-restart-governance.md"
CROSS_LINK_DOCS = (
REPO_ROOT / "docs" / "safety-model.md",
REPO_ROOT / "docs" / "webui-deployment.md",
)
LINKED_ISSUES = ("#655", "#652", "#653", "#630", "#642")
POLICY_IDS = ("RG-01", "RG-02", "RG-03", "RG-04", "RG-05", "RG-06", "RG-07", "RG-08")
def _read(path: Path) -> str:
assert path.is_file(), f"missing {path.relative_to(REPO_ROOT)}"
return path.read_text(encoding="utf-8")
def test_ac1_adr_exists_with_matrix_and_v1_decision():
text = _read(ADR)
lower = text.lower()
assert text.lstrip().startswith("#"), "ADR lacks a title"
assert "#656" in text
assert "authorization matrix" in lower
# The matrix is a real table with the worker and privileged roles.
for role in ("author", "reviewer", "merger", "reconciler", "controller",
"operator", "admin"):
assert role in lower, f"authorization matrix missing role {role!r}"
# Recorded v1 decision.
assert "restart-governance/v1" in text
assert "controller approval" in lower and "automated safety gates" in lower
def test_ac2_restart_is_last_resort_with_narrower_recoveries():
text = _read(ADR)
lower = text.lower()
assert "last resort" in lower
# Enumerated narrower recoveries precede full restart on the ladder.
for rung in ("reconnect", "rebind", "scoped restart", "full restart",
"host"):
assert rung in lower, f"recovery ladder missing rung {rung!r}"
def test_ac3_forbids_unilateral_llm_full_restart_with_affected_sessions():
text = _read(ADR)
lower = text.lower()
assert "forbidden" in lower
assert "llm" in lower and "restart" in lower
assert "unilateral" in lower
# A worker role must not perform or authorize full/host restart.
assert "must not" in lower
def test_ac4_break_glass_conditions_listed():
text = _read(ADR)
lower = text.lower()
assert "break-glass" in lower
assert "incident" in lower
assert "audit" in lower
def test_ac5_adr_links_issue_lineage():
text = _read(ADR)
for issue in LINKED_ISSUES:
assert issue in text, f"ADR must link issue {issue}"
def test_ac5_safety_model_and_deployment_cross_link_adr():
for path in CROSS_LINK_DOCS:
text = _read(path)
assert ADR_BASENAME in text, (
f"{path.relative_to(REPO_ROOT)} must cross-link {ADR_BASENAME} "
f"(issue #656 acceptance criterion 5)"
)
def test_policy_ids_present_for_enforcement_code():
text = _read(ADR)
for pid in POLICY_IDS:
assert pid in text, f"policy id {pid} missing from ADR"
def test_failure_behavior_denies_on_ambiguity():
text = _read(ADR)
lower = text.lower()
assert "ambiguous" in lower and "deny" in lower
def test_cross_links_do_not_embed_secrets():
for path in (ADR,) + CROSS_LINK_DOCS:
text = _read(path)
for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"):
assert marker not in text, f"{path} contains {marker!r}"
+345
View File
@@ -0,0 +1,345 @@
"""Tests for the system-health dashboard view (#639).
Covers the acceptance criteria directly: the page renders the health DTO
fields (AC1), degraded dependencies are visible (AC2), stale runtime is warned
prominently and never rendered as mutation-safe (AC3), healthy and degraded
fixtures both render (AC4), and the shell carries a nav entry (AC5).
"""
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from starlette.testclient import TestClient
from webui.app import create_app
from webui.deployment_boundary import scan_text_for_client_secrets
from webui.layout import render_page
from webui.nav import iter_nav_items
from webui.system_health import (
STATUS_DEGRADED,
STATUS_DOWN,
STATUS_OK,
STATUS_SKIPPED,
STATUS_UNPROVEN,
DependencyProbe,
StaleRuntime,
SystemHealthSnapshot,
VersionInfo,
)
from webui.system_health_views import render_system_health_page
DASHBOARD_PATH = "/system-health"
def _version(*, known: bool = True) -> VersionInfo:
return VersionInfo(
git_sha="1c455b6ec0f9cb761fe6248de68c17e061fb5ecd" if known else None,
git_describe="v0.4.1-12-g1c455b6" if known else None,
control_plane_schema_version=4 if known else None,
python_version="3.13.1",
known=known,
)
def _parity(*, stale: bool = False, determinable: bool = True) -> StaleRuntime:
if stale:
return StaleRuntime(
daemon_head="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
checkout_head="bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
remote_head="cccccccccccccccccccccccccccccccccccccccc",
stale=True,
determinable=True,
mutation_safe=False,
reasons=("runtime, checkout, and remote commits disagree",),
)
if not determinable:
return StaleRuntime(
daemon_head=None,
checkout_head=None,
remote_head=None,
stale=False,
determinable=False,
mutation_safe=False,
reasons=("local checkout HEAD could not be read",),
)
return StaleRuntime(
daemon_head="1c455b6ec0f9cb761fe6248de68c17e061fb5ecd",
checkout_head="1c455b6ec0f9cb761fe6248de68c17e061fb5ecd",
remote_head="1c455b6ec0f9cb761fe6248de68c17e061fb5ecd",
stale=False,
determinable=True,
mutation_safe=True,
reasons=(),
)
def _snapshot(
*,
status: str = STATUS_OK,
ready: bool = True,
readiness_complete: bool = True,
readiness_reasons: tuple[str, ...] = (),
dependencies: tuple[DependencyProbe, ...] | None = None,
parity: StaleRuntime | None = None,
namespaces: tuple[dict, ...] = (),
probe_errors: tuple[str, ...] = (),
version_known: bool = True,
) -> SystemHealthSnapshot:
if dependencies is None:
dependencies = (
DependencyProbe(
name="control_plane_db",
kind="sqlite",
status=STATUS_OK,
detail="schema version 4",
required=True,
latency_ms=1.25,
metadata={"schema_version": 4},
),
)
return SystemHealthSnapshot(
status=status,
ready=ready,
readiness_complete=readiness_complete,
readiness_reasons=readiness_reasons,
service="mcp-control-plane-webui",
mode="read-only",
version=_version(known=version_known),
started_at="2026-07-23T19:50:47+00:00",
uptime_seconds=3661.5,
timestamp="2026-07-23T20:51:48+00:00",
deep_probes_requested=False,
dependencies=dependencies,
mcp_namespaces=namespaces,
stale_runtime=parity if parity is not None else _parity(),
probe_errors=probe_errors,
)
class TestHealthyRender(unittest.TestCase):
"""AC1 / AC4 — every health DTO field reaches the page."""
def setUp(self):
self.html = render_system_health_page(_snapshot())
def test_readiness_fields_render(self):
self.assertIn("System health", self.html)
self.assertIn("Ready", self.html)
self.assertIn("mcp-control-plane-webui", self.html)
self.assertIn("read-only", self.html)
self.assertIn("2026-07-23T20:51:48+00:00", self.html)
def test_version_and_uptime_render(self):
self.assertIn("1c455b6ec0f9cb761fe6248de68c17e061fb5ecd", self.html)
self.assertIn("v0.4.1-12-g1c455b6", self.html)
self.assertIn("3.13.1", self.html)
self.assertIn("3661.500s", self.html)
self.assertIn("1.02h", self.html)
def test_dependency_row_renders_with_latency(self):
self.assertIn("control_plane_db", self.html)
self.assertIn("sqlite", self.html)
self.assertIn("schema version 4", self.html)
self.assertIn("1.2 ms", self.html)
def test_healthy_page_shows_no_stale_warning(self):
self.assertNotIn("Stale runtime:", self.html)
self.assertNotIn("Staleness", self.html)
def test_unknown_version_is_labelled_not_faked(self):
html = render_system_health_page(_snapshot(version_known=False))
self.assertIn("unknown", html)
self.assertIn("unresolved", html)
class TestDegradedRender(unittest.TestCase):
"""AC2 — a degraded or unrun dependency is visible, not swallowed."""
def setUp(self):
self.deps = (
DependencyProbe(
name="control_plane_db",
kind="sqlite",
status=STATUS_OK,
detail="schema version 4",
required=True,
latency_ms=0.9,
),
DependencyProbe(
name="repository",
kind="git",
status=STATUS_DOWN,
detail="repository root is not a git checkout",
required=True,
latency_ms=4.0,
),
DependencyProbe(
name="gitea",
kind="http",
status=STATUS_SKIPPED,
detail="deep probe not requested",
required=False,
),
)
self.html = render_system_health_page(
_snapshot(
status=STATUS_DEGRADED,
ready=False,
readiness_complete=False,
readiness_reasons=("required dependency 'repository' is down",),
dependencies=self.deps,
)
)
def test_degraded_banner_names_the_dependency(self):
self.assertIn("Degraded dependencies:", self.html)
self.assertIn("repository", self.html)
def test_not_run_probe_is_reported_separately(self):
self.assertIn("Not probed:", self.html)
self.assertIn("gitea", self.html)
self.assertIn("not counted", self.html)
def test_not_ready_headline_and_reason(self):
self.assertIn("Not ready", self.html)
self.assertIn("required dependency &#x27;repository&#x27; is down", self.html)
def test_degraded_status_badge_present(self):
self.assertIn("badge-health-degraded", self.html)
self.assertIn("badge-health-down", self.html)
def test_ready_but_incomplete_is_not_shown_as_plain_ready(self):
html = render_system_health_page(
_snapshot(ready=True, readiness_complete=False)
)
self.assertIn("Ready (incomplete evidence)", html)
class TestStaleRuntimeWarning(unittest.TestCase):
"""AC3 — staleness is prominent and never claims mutation safety."""
def test_stale_runtime_warns_and_denies_mutation_safety(self):
html = render_system_health_page(_snapshot(parity=_parity(stale=True)))
self.assertIn("Stale runtime:", html)
self.assertIn("do not treat this runtime as mutation-safe", html)
self.assertIn("<tr><th>Mutation safe</th><td>False</td></tr>", html)
def test_indeterminate_parity_is_not_reported_safe(self):
html = render_system_health_page(
_snapshot(parity=_parity(determinable=False))
)
self.assertIn("Staleness", html)
self.assertIn("<tr><th>Mutation safe</th><td>False</td></tr>", html)
self.assertIn("<tr><th>Determinable</th><td>False</td></tr>", html)
def test_healthy_parity_reports_mutation_safe_true(self):
html = render_system_health_page(_snapshot())
self.assertIn("<tr><th>Mutation safe</th><td>True</td></tr>", html)
class TestNamespacesAndErrors(unittest.TestCase):
def test_unproven_namespace_rows_render(self):
html = render_system_health_page(
_snapshot(
namespaces=(
{
"namespace": "gitea-author",
"required_tool": "gitea_lock_issue",
"status": STATUS_UNPROVEN,
"ide_namespace_proven": False,
"reason": "the web console cannot invoke the IDE-managed MCP client",
},
)
)
)
self.assertIn("gitea-author", html)
self.assertIn("gitea_lock_issue", html)
self.assertIn("badge-health-unproven", html)
def test_no_namespaces_degrades_gracefully(self):
html = render_system_health_page(_snapshot(namespaces=()))
self.assertIn("No MCP namespaces are declared.", html)
def test_probe_errors_render_when_present(self):
html = render_system_health_page(
_snapshot(probe_errors=("probe raised: disk offline",))
)
self.assertIn("Probe errors", html)
self.assertIn("disk offline", html)
def test_probe_error_card_absent_when_clean(self):
self.assertNotIn("Probe errors", render_system_health_page(_snapshot()))
class TestReadOnlyAndRedaction(unittest.TestCase):
def test_no_restart_or_kill_controls(self):
html = render_system_health_page(_snapshot())
self.assertNotIn("<button", html)
self.assertNotIn("<form", html)
self.assertNotIn("pkill", html)
self.assertIn("read-only", html)
def test_recovery_points_at_sanctioned_path(self):
html = render_system_health_page(_snapshot())
self.assertIn("Reconnect the MCP client", html)
self.assertIn("Never kill the daemon process manually", html)
def test_secret_shaped_detail_is_redacted(self):
leaky = DependencyProbe(
name="gitea",
kind="http",
status=STATUS_DOWN,
detail="auth failed for token=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789",
required=False,
latency_ms=12.0,
)
html = render_system_health_page(_snapshot(dependencies=(leaky,)))
self.assertNotIn("ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789", html)
def test_html_in_detail_is_escaped(self):
hostile = DependencyProbe(
name="repository",
kind="git",
status=STATUS_DOWN,
detail="<script>alert(1)</script>",
required=True,
)
html = render_system_health_page(_snapshot(dependencies=(hostile,)))
self.assertNotIn("<script>", html)
self.assertIn("&lt;script&gt;", html)
class TestNavAndRoute(unittest.TestCase):
"""AC5 — the shell links the dashboard, and the route serves it."""
def setUp(self):
self.client = TestClient(create_app())
def test_nav_contains_system_health(self):
self.assertIn(
(DASHBOARD_PATH, "System health"),
[(item.href, item.label) for item in iter_nav_items()],
)
def test_rendered_shell_links_dashboard(self):
page = render_page(title="Home", body_html="<p>x</p>")
self.assertIn(f'href="{DASHBOARD_PATH}"', page)
def test_route_renders_dashboard(self):
response = self.client.get(DASHBOARD_PATH)
self.assertEqual(response.status_code, 200)
self.assertIn("System health", response.text)
self.assertIn("Stale-runtime parity", response.text)
def test_route_is_read_only(self):
self.assertEqual(self.client.post(DASHBOARD_PATH).status_code, 405)
def test_live_page_leaks_no_client_secret(self):
findings = scan_text_for_client_secrets(self.client.get(DASHBOARD_PATH).text)
self.assertEqual(findings, [])
if __name__ == "__main__": # pragma: no cover
unittest.main()
+20
View File
@@ -53,6 +53,7 @@ from webui.system_health import (
process_uptime, process_uptime,
snapshot_to_dict as system_health_to_dict, snapshot_to_dict as system_health_to_dict,
) )
from webui.system_health_views import render_system_health_page
_READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) _READ_ONLY_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
_AUDIT_MUTATION_PATHS = frozenset({"/audit", "/api/audit"}) _AUDIT_MUTATION_PATHS = frozenset({"/audit", "/api/audit"})
@@ -161,6 +162,24 @@ async def api_system_health(request: Request) -> JSONResponse:
return JSONResponse(payload, status_code=200 if snapshot.ready else 503) return JSONResponse(payload, status_code=200 if snapshot.ready else 503)
async def system_health(request: Request) -> HTMLResponse:
"""Read-only system-health dashboard (#639).
Shares the #634 snapshot loader with the JSON API so the page can never
disagree with it. `?deep=1` opts into the network probe exactly as the API
does; the default page load stays cheap. The response is always 200: this
is an operator view that must render the degraded state, not withhold it.
"""
deep = _truthy_flag(request.query_params.get("deep"))
snapshot = load_system_health(deep=deep)
return HTMLResponse(
render_page(
title="System health",
body_html=render_system_health_page(snapshot),
)
)
async def queue(_request: Request) -> HTMLResponse: async def queue(_request: Request) -> HTMLResponse:
snapshot = load_queue_snapshot() snapshot = load_queue_snapshot()
return HTMLResponse(render_page(title="Queue", body_html=render_queue_page(snapshot))) return HTMLResponse(render_page(title="Queue", body_html=render_queue_page(snapshot)))
@@ -571,6 +590,7 @@ def create_app(*, bind_host: str | None = None) -> Starlette:
Route("/", home, methods=["GET"]), Route("/", home, methods=["GET"]),
Route("/health", health, methods=["GET"]), Route("/health", health, methods=["GET"]),
Route(SYSTEM_HEALTH_API_PATH, api_system_health, methods=["GET"]), Route(SYSTEM_HEALTH_API_PATH, api_system_health, methods=["GET"]),
Route("/system-health", system_health, methods=["GET"]),
Route("/queue", queue, methods=["GET"]), Route("/queue", queue, methods=["GET"]),
Route("/api/queue", api_queue, methods=["GET"]), Route("/api/queue", api_queue, methods=["GET"]),
Route("/projects", projects, methods=["GET"]), Route("/projects", projects, methods=["GET"]),
+19
View File
@@ -236,6 +236,25 @@ def render_page(*, title: str, body_html: str, extra_head: str = "") -> str:
.badge-in-review {{ color: #9ec8f0; border-color: #3d5f7a; }} .badge-in-review {{ color: #9ec8f0; border-color: #3d5f7a; }}
.badge-duplicate {{ color: #e0c27a; border-color: #6b5730; }} .badge-duplicate {{ color: #e0c27a; border-color: #6b5730; }}
.badge-stale {{ color: #c9b8e8; border-color: #5a4a78; }} .badge-stale {{ color: #c9b8e8; border-color: #5a4a78; }}
.badge-health-ok {{ color: #8fd19e; border-color: #3d6b4a; }}
.badge-health-degraded {{ color: #e0c27a; border-color: #6b5730; }}
.badge-health-down {{ color: #f0a8a8; border-color: #7a3b3b; }}
.badge-health-skipped {{ color: var(--muted); }}
.badge-health-unproven {{ color: #c9b8e8; border-color: #5a4a78; }}
.health-card {{
margin: 1.25rem 0;
padding: 0.85rem 1rem 1rem;
border: 1px solid var(--border);
border-radius: 8px;
background: var(--surface);
}}
.health-card h3 {{ margin: 0 0 0.5rem; font-size: 1.05rem; }}
.health-card h4 {{ margin: 1rem 0 0.35rem; font-size: 0.92rem; color: var(--muted); }}
.health-headline {{ color: var(--text); font-size: 1rem; margin: 0 0 0.5rem; }}
.health-degraded {{ border-left-color: #e0c27a; }}
.health-stale {{ border-left-color: #f0a8a8; }}
ul.reasons {{ margin: 0.35rem 0; padding-left: 1.15rem; color: var(--muted); font-size: 0.9rem; }}
ul.reasons li {{ margin-bottom: 0.3rem; }}
</style> </style>
{extra_head} {extra_head}
</head> </head>
+1
View File
@@ -38,6 +38,7 @@ class NavGroup:
NAV_GROUPS: tuple[NavGroup, ...] = ( NAV_GROUPS: tuple[NavGroup, ...] = (
NavGroup("Health", ( NavGroup("Health", (
NavItem("/health", "Liveness"), NavItem("/health", "Liveness"),
NavItem("/system-health", "System health"),
)), )),
NavGroup("Traffic", ( NavGroup("Traffic", (
NavItem("/queue", "Queue"), NavItem("/queue", "Queue"),
+307
View File
@@ -0,0 +1,307 @@
"""HTML views for the system-health dashboard (#639).
Renders the read-only :class:`~webui.system_health.SystemHealthSnapshot`
produced by the Phase 1 system-health API (#634). The page offers no restart,
reload, or process-kill control: those are Phase 2 work, and manual process
kills are the contamination path #630 exists to prevent.
Every free-text field passes through :func:`webui.system_health.redact` before
it reaches HTML, so a probe detail that captured a token or a credentialed URL
cannot leak through the dashboard even though the API redacts it already.
"""
from __future__ import annotations
import html
from webui.system_health import (
STATUS_DEGRADED,
STATUS_DOWN,
STATUS_OK,
STATUS_SKIPPED,
STATUS_UNPROVEN,
DependencyProbe,
SystemHealthSnapshot,
redact,
)
_STATUS_BADGE_CLASS = {
STATUS_OK: "badge-health-ok",
STATUS_DEGRADED: "badge-health-degraded",
STATUS_DOWN: "badge-health-down",
STATUS_SKIPPED: "badge-health-skipped",
STATUS_UNPROVEN: "badge-health-unproven",
}
def _safe(value: object) -> str:
"""Escape free text for HTML after redacting anything secret-shaped.
Use this for every value that can carry arbitrary text probe details,
reasons, probe errors because those are where a credential could ride
along.
"""
return html.escape(redact(str(value)))
def _esc(value: object) -> str:
"""Escape a structured field for HTML without redacting it.
Commit SHAs, probe names, statuses, and timestamps are enumerated or
machine-generated, never credential-bearing. They must not go through
:func:`redact`: its opaque-token rule matches any 32-plus-character run,
so a 40-character git SHA would render as ``[redacted]`` and the parity
view the one thing an operator reads this page for would be blank.
"""
return html.escape(str(value))
def _status_badge(status: str) -> str:
css = _STATUS_BADGE_CLASS.get(status, "badge-health-unproven")
return f'<span class="badge {css}">{_esc(status)}</span>'
def _reason_list(reasons: tuple[str, ...], *, empty: str) -> str:
if not reasons:
return f"<p class='muted'>{html.escape(empty)}</p>"
items = "".join(f"<li>{_safe(reason)}</li>" for reason in reasons)
return f"<ul class='reasons'>{items}</ul>"
def _readiness_card(snapshot: SystemHealthSnapshot) -> str:
"""Overall readiness.
``ready`` and ``readiness_complete`` are shown separately on purpose: a
snapshot whose required probes never ran is not the same as one that ran
them and passed, and collapsing the two would render an unproven green.
"""
if snapshot.ready and snapshot.readiness_complete:
headline = "Ready"
elif snapshot.ready:
headline = "Ready (incomplete evidence)"
else:
headline = "Not ready"
return (
"<section class='health-card'>"
f"<h3>Readiness {_status_badge(snapshot.status)}</h3>"
f"<p class='health-headline'>{html.escape(headline)}</p>"
"<table class='detail'>"
f"<tr><th>Service</th><td><code>{_esc(snapshot.service)}</code></td></tr>"
f"<tr><th>Mode</th><td>{_esc(snapshot.mode)}</td></tr>"
f"<tr><th>Ready</th><td>{_esc(snapshot.ready)}</td></tr>"
"<tr><th>Readiness evidence complete</th>"
f"<td>{_esc(snapshot.readiness_complete)}</td></tr>"
"<tr><th>Deep probes requested</th>"
f"<td>{_esc(snapshot.deep_probes_requested)}</td></tr>"
f"<tr><th>Observed at</th><td><code>{_esc(snapshot.timestamp)}</code></td></tr>"
"</table>"
"<h4>Readiness reasons</h4>"
f"{_reason_list(snapshot.readiness_reasons, empty='No readiness objections recorded.')}"
"</section>"
)
def _version_card(snapshot: SystemHealthSnapshot) -> str:
version = snapshot.version
uptime_hours = snapshot.uptime_seconds / 3600.0
known = (
"resolved"
if version.known
else "unresolved — version fields could not be read from the checkout"
)
schema = version.control_plane_schema_version
return (
"<section class='health-card'>"
"<h3>Version and uptime</h3>"
"<table class='detail'>"
f"<tr><th>Git SHA</th><td><code>{_esc(version.git_sha or 'unknown')}</code></td></tr>"
"<tr><th>Git describe</th>"
f"<td><code>{_esc(version.git_describe or 'unknown')}</code></td></tr>"
"<tr><th>Control-plane schema</th>"
f"<td>{_esc(schema if schema is not None else 'unknown')}</td></tr>"
f"<tr><th>Python</th><td><code>{_esc(version.python_version)}</code></td></tr>"
f"<tr><th>Version status</th><td>{html.escape(known)}</td></tr>"
f"<tr><th>Started at</th><td><code>{_esc(snapshot.started_at)}</code></td></tr>"
"<tr><th>Uptime</th>"
f"<td>{snapshot.uptime_seconds:.3f}s ({uptime_hours:.2f}h)</td></tr>"
"</table>"
"</section>"
)
def _dependency_rows(probes: tuple[DependencyProbe, ...]) -> str:
if not probes:
return "<p class='muted'>No dependency probes were reported.</p>"
rows = []
for probe in probes:
latency = (
f"{probe.latency_ms:.1f} ms" if probe.latency_ms is not None else "n/a"
)
rows.append(
"<tr>"
f"<td><code>{_esc(probe.name)}</code></td>"
f"<td>{_esc(probe.kind)}</td>"
f"<td>{_status_badge(probe.status)}</td>"
f"<td>{_esc('required' if probe.required else 'optional')}</td>"
f"<td>{html.escape(latency)}</td>"
f"<td>{_safe(probe.detail)}</td>"
"</tr>"
)
return (
"<table class='registry'><thead><tr>"
"<th>Dependency</th><th>Kind</th><th>Status</th><th>Requirement</th>"
"<th>Latency</th><th>Detail</th>"
"</tr></thead><tbody>"
f"{''.join(rows)}</tbody></table>"
)
def _dependency_card(snapshot: SystemHealthSnapshot) -> str:
degraded = [probe for probe in snapshot.dependencies if probe.ran and not probe.healthy]
not_run = [probe for probe in snapshot.dependencies if not probe.ran]
banner = ""
if degraded:
names = ", ".join(sorted(probe.name for probe in degraded))
banner += (
"<div class='stub health-degraded'><p><strong>Degraded dependencies:</strong> "
f"{_esc(names)}</p></div>"
)
if not_run:
names = ", ".join(sorted(probe.name for probe in not_run))
banner += (
"<div class='stub'><p><strong>Not probed:</strong> "
f"{_esc(names)} — these contribute no evidence and are not counted "
"as healthy.</p></div>"
)
return (
"<section class='health-card'>"
"<h3>Dependencies</h3>"
f"{banner}"
f"{_dependency_rows(snapshot.dependencies)}"
"<p class='muted'>Details are redacted at the API boundary and again "
"before rendering; credentials are never displayed.</p>"
"</section>"
)
def _namespace_card(snapshot: SystemHealthSnapshot) -> str:
if not snapshot.mcp_namespaces:
body = "<p class='muted'>No MCP namespaces are declared.</p>"
else:
rows = []
for entry in snapshot.mcp_namespaces:
rows.append(
"<tr>"
f"<td><code>{_esc(entry.get('namespace'))}</code></td>"
f"<td><code>{_esc(entry.get('required_tool'))}</code></td>"
f"<td>{_status_badge(str(entry.get('status') or STATUS_UNPROVEN))}</td>"
f"<td>{_esc(entry.get('ide_namespace_proven'))}</td>"
f"<td>{_safe(entry.get('reason'))}</td>"
"</tr>"
)
body = (
"<table class='registry'><thead><tr>"
"<th>Namespace</th><th>Required tool</th><th>Status</th>"
"<th>IDE-proven</th><th>Reason</th>"
"</tr></thead><tbody>"
f"{''.join(rows)}</tbody></table>"
)
return (
"<section class='health-card'>"
"<h3>MCP namespaces</h3>"
f"{body}"
"<p class='muted'>The web process runs outside the IDE-managed MCP "
"client, so namespace health is reported as unproven rather than "
"guessed (#543).</p>"
"</section>"
)
def _stale_runtime_card(snapshot: SystemHealthSnapshot) -> str:
stale = snapshot.stale_runtime
if stale.stale:
warning = (
"<div class='stub health-stale'><p><strong>Stale runtime:</strong> "
"the running code, the checkout, and the remote-tracking commit "
"disagree. Capability gates may be evaluating obsolete code — "
"do not treat this runtime as mutation-safe.</p></div>"
)
elif not stale.determinable:
warning = (
"<div class='stub health-stale'><p><strong>Staleness "
"indeterminate:</strong> parity could not be proven, so this "
"runtime is not reported as mutation-safe.</p></div>"
)
else:
warning = ""
return (
"<section class='health-card'>"
"<h3>Stale-runtime parity</h3>"
f"{warning}"
"<table class='detail'>"
"<tr><th>Daemon head</th>"
f"<td><code>{_esc(stale.daemon_head or 'unknown')}</code></td></tr>"
"<tr><th>Checkout head</th>"
f"<td><code>{_esc(stale.checkout_head or 'unknown')}</code></td></tr>"
"<tr><th>Remote head</th>"
f"<td><code>{_esc(stale.remote_head or 'unknown')}</code></td></tr>"
f"<tr><th>Stale</th><td>{_esc(stale.stale)}</td></tr>"
f"<tr><th>Determinable</th><td>{_esc(stale.determinable)}</td></tr>"
f"<tr><th>Mutation safe</th><td>{_esc(stale.mutation_safe)}</td></tr>"
"</table>"
f"{_reason_list(stale.reasons, empty='Runtime, checkout, and remote agree.')}"
"</section>"
)
def _probe_error_card(snapshot: SystemHealthSnapshot) -> str:
if not snapshot.probe_errors:
return ""
return (
"<section class='health-card'>"
"<h3>Probe errors</h3>"
f"{_reason_list(snapshot.probe_errors, empty='')}"
"</section>"
)
def _recovery_card() -> str:
"""Sanctioned recovery pointers only — never a manual process kill (#630)."""
return (
"<section class='health-card'>"
"<h3>Recovery</h3>"
"<p class='muted'>This dashboard is read-only. Restart and reload "
"controls arrive in Phase 2 (#642); until then recovery runs through "
"the sanctioned client reconnect / operator restart path.</p>"
"<ul class='reasons'>"
"<li><a href='/runtime'>Runtime and session view</a> — active profile, "
"workflow hashes, and shell health.</li>"
"<li>Reconnect the MCP client from the IDE, then re-run the blocked "
"cycle. Never kill the daemon process manually: unmanaged kills are "
"recorded as runtime contamination (#630).</li>"
"<li>See <code>docs/webui-local-dev.md</code> for the documented "
"recovery sequence.</li>"
"</ul>"
"</section>"
)
def render_system_health_page(snapshot: SystemHealthSnapshot) -> str:
"""Render the full system-health dashboard body."""
return (
"<h2>System health</h2>"
"<p class='meta'>Read-only view of the Phase 1 system-health API "
"(<code>/api/v1/system/health</code>). Reload this page to refresh; "
"nothing here polls or mutates on your behalf.</p>"
f"{_readiness_card(snapshot)}"
f"{_stale_runtime_card(snapshot)}"
f"{_version_card(snapshot)}"
f"{_dependency_card(snapshot)}"
f"{_namespace_card(snapshot)}"
f"{_probe_error_card(snapshot)}"
f"{_recovery_card()}"
)