From 1301a57de4626118cc2a0d81ce6eb09da22ac3ac Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 23 Jul 2026 19:16:28 -0400 Subject: [PATCH 1/3] docs(governance): MCP restart governance and authorization policy (#656) Adds docs/architecture/mcp-restart-governance.md, the restart-governance/v1 ADR defining who may restart the MCP control plane and under what conditions. - Recovery ladder (reconnect -> rebind -> scoped restart -> full restart -> host) with restart stated as the last resort. - Authorization matrix across author/reviewer/merger/reconciler/controller/ operator/admin; no LLM worker role may perform or authorize a full or host restart. - v1 authority decision recorded: controller approval + automated safety gates; quorum deferred to a superseding ADR. - Break-glass path with pre-declared incident and mandatory post-hoc audit. - Ambiguous policy state denies restart. - Stable policy IDs RG-01..RG-08 for later enforcement code to bind to. Cross-links the ADR from docs/safety-model.md and docs/webui-deployment.md, and adds tests/test_mcp_restart_governance_docs.py asserting acceptance criteria 1-5. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/architecture/mcp-restart-governance.md | 222 ++++++++++++++++++++ docs/safety-model.md | 14 ++ docs/webui-deployment.md | 9 + tests/test_mcp_restart_governance_docs.py | 107 ++++++++++ 4 files changed, 352 insertions(+) create mode 100644 docs/architecture/mcp-restart-governance.md create mode 100644 tests/test_mcp_restart_governance_docs.py diff --git a/docs/architecture/mcp-restart-governance.md b/docs/architecture/mcp-restart-governance.md new file mode 100644 index 0000000..da8fbd4 --- /dev/null +++ b/docs/architecture/mcp-restart-governance.md @@ -0,0 +1,222 @@ +# 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) — MCP restart / health hardening umbrella + - Vision: [#652](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/652) — control-plane restart vision + - Roadmap: [#653](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/653) — restart hardening roadmap + - Coordinator: [#630](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/630) — forbids process-kill recovery + - Break-glass / console approval: [#642](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/642) + - Transport recovery: [#591](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/591), [#584](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/584) — EOF / transport-flap self-recovery + - 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 1–2 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 (#630 coordinator, #642 console approval) binds to these stable +policy identifiers rather than to prose: + +| Policy ID | Statement | +|---|---| +| `RG-01` | Restart is last resort; rungs 1–2 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**, coordinator **#630**, and break-glass / console approval **#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. diff --git a/docs/safety-model.md b/docs/safety-model.md index 31c740a..bbab241 100644 --- a/docs/safety-model.md +++ b/docs/safety-model.md @@ -46,3 +46,17 @@ 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 [`llm-workflow-runbooks.md`](llm-workflow-runbooks.md) § MCP-native commit path (#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. diff --git a/docs/webui-deployment.md b/docs/webui-deployment.md index fa754ad..2ad46ab 100644 --- a/docs/webui-deployment.md +++ b/docs/webui-deployment.md @@ -55,6 +55,15 @@ shipped to the browser. assumption paths, and the client-secret policy. Use it to verify an instance is 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) - Full SSO or session login in the UI diff --git a/tests/test_mcp_restart_governance_docs.py b/tests/test_mcp_restart_governance_docs.py new file mode 100644 index 0000000..5b12e50 --- /dev/null +++ b/tests/test_mcp_restart_governance_docs.py @@ -0,0 +1,107 @@ +"""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}" From 9f759150b8bde333e123bcd8efd302d4ebd809c1 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Thu, 23 Jul 2026 19:18:47 -0400 Subject: [PATCH 2/3] docs(governance): correct linked-issue descriptions in restart ADR (#656) The Related section and cross-reference lines described #652, #653, #630, #642, and #591 by roles they do not hold. Align each description with the linked issue's actual title and scope: - #652 is the Control Plane Web Console product vision (restart controls live in its capability area A), not a restart-specific vision. - #653 is the console phased-delivery roadmap; restart controls are Phase 2. - #630 is the manual process-kill contamination guard, not the coordinator; the coordinator remains an unimplemented later child of #655. - #642 is the sanctioned restart / graceful reload console UX. - #591 is auto-restart on master advance (closed); only #584 is transport-flap reconnect. They were previously collapsed into one transport-recovery label. Documentation-only wording change. Policy IDs RG-01..RG-08, the policy version restart-governance/v1, the authorization matrix, and every normative statement are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/architecture/mcp-restart-governance.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/architecture/mcp-restart-governance.md b/docs/architecture/mcp-restart-governance.md index da8fbd4..2931d10 100644 --- a/docs/architecture/mcp-restart-governance.md +++ b/docs/architecture/mcp-restart-governance.md @@ -5,12 +5,12 @@ - **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) — MCP restart / health hardening umbrella - - Vision: [#652](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/652) — control-plane restart vision - - Roadmap: [#653](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/653) — restart hardening roadmap - - Coordinator: [#630](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/630) — forbids process-kill recovery - - Break-glass / console approval: [#642](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/642) - - Transport recovery: [#591](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/591), [#584](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/584) — EOF / transport-flap self-recovery + - 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` @@ -175,7 +175,8 @@ never to restart on assumption. ## 5. Policy IDs (for enforcement code) -Enforcement code (#630 coordinator, #642 console approval) binds to these stable +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 | @@ -210,7 +211,7 @@ cross-linked from the safety model and the web-console deployment boundary: - `docs/webui-deployment.md` references this ADR for restart/reload disposition. It is linked to its issue lineage — umbrella **#655**, vision **#652**, roadmap -**#653**, coordinator **#630**, and break-glass / console approval **#642** — in +**#653**, contamination guard **#630**, and console restart UX **#642** — in § Related above. ## 8. Non-goals From 5d59c57c98a5c8709bbbe1d5d5ed13eab721b14d Mon Sep 17 00:00:00 2001 From: Jason Walker Date: Fri, 24 Jul 2026 02:56:57 -0400 Subject: [PATCH 3/3] fix(author): refresh durable issue-lock head after branch sync; recover merge-sync-drifted dead-session locks (#871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gitea_update_pr_branch_by_merge` advanced a PR's remote head but 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). Write-side (prevents future drift): - issue_lock_store.assess/apply_durable_lock_head_refresh: on a successful sync, CAS-refresh the durable lock's recorded head from the exact expected PR head to the resulting head, re-verifying repo/issue/branch/worktree/ identity/profile/live-session ownership, with read-after-write verification. - gitea_update_pr_branch_by_merge now refreshes the lock after the remote advance and reports a PARTIAL LIFECYCLE FAILURE (success=False) when the refresh fails, instead of falsely reporting a full synchronization. Read-side (recovers already-drifted locks): - issue_lock_worktree.read_merge_sync_provenance: server-side git observation proving a remote head is a sanctioned base-into-branch merge that preserved the branch mainline back to the recorded head. - issue_lock_recovery: new HEAD_RELATION_REMOTE_MERGE_SYNCED accepts a dead-session lock whose recorded head is a strict merge-sync ancestor of the live PR head — and only that. Rewrites, rebases, force-pushes, non-ancestor heads, dirty worktrees, live/competing owners, and wrong repo/issue/branch/ identity/profile all stay protected. No existing exact-head, branch-protection, parity, workspace, identity, role, or mutation-safety gate is weakened. All provenance is server-derived; nothing is reachable from an MCP caller. Tests: tests/test_issue_871_durable_lock_head_refresh.py (32 cases) covering first/second sync, CAS, ownership, partial-failure, merge-sync recovery happy-path and every fail-closed branch. Full suite: 4789 passed, 13 pre- existing baseline failures unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01FZPyVh2DGczQrDxtqwGH5p --- gitea_mcp_server.py | 100 ++- issue_lock_recovery.py | 138 +++- issue_lock_store.py | 306 ++++++++- issue_lock_worktree.py | 169 +++++ ...est_issue_871_durable_lock_head_refresh.py | 630 ++++++++++++++++++ 5 files changed, 1325 insertions(+), 18 deletions(-) create mode 100644 tests/test_issue_871_durable_lock_head_refresh.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 8e1661a..fb50976 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -2445,6 +2445,20 @@ def _evaluate_issue_lock_recovery( 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 # base the branch was cut from is observed instead. Probed only in that # case, so the published path's evidence is untouched (#772 AC8). @@ -2487,6 +2501,7 @@ def _evaluate_issue_lock_recovery( remote_branch_exists=remote_branch_exists, recorded_base_sha=recorded_base, base_ancestry=base_ancestry, + sync_provenance=sync_provenance, ) @@ -18889,10 +18904,59 @@ def gitea_update_pr_branch_by_merge( prepared_verdict_head_sha=live_pr_head, ) - return { - "success": True, + # #871: the remote head is now advanced; the durable linked-issue lock must + # be advanced with it, or a later dead-session recovery can never prove + # 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, "mutation_allowed": True, + "durable_lock_refreshed": durable_refreshed, + "durable_lock_refresh": lock_refresh, + "fully_synchronized": durable_refreshed, "style": "merge", "force_push": False, "rebase": False, @@ -18914,19 +18978,37 @@ def gitea_update_pr_branch_by_merge( "prepared_verdict_invalidated": transition.get( "prepared_verdict_invalidated" ), - "recommended_next_action": transition.get( - "recommended_next_action", - pr_sync_status.ACTION_FRESH_REVIEW_REQUIRED, - ), "transition": transition, "role_kind": role, "profile_name": profile.get("profile_name"), "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() def gitea_assess_conflict_fix_push( diff --git a/issue_lock_recovery.py b/issue_lock_recovery.py index ecc99c6..ae0cc35 100644 --- a/issue_lock_recovery.py +++ b/issue_lock_recovery.py @@ -85,6 +85,12 @@ HEAD_RELATION_STRICT_DESCENDANT = "strict_descendant" # #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. 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 # interchangeable: a published claim proves ownership against a remote/PR head, @@ -266,6 +272,70 @@ 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( existing_lock: Mapping[str, Any] | None, *, @@ -290,6 +360,7 @@ def assess_dead_session_lock_recovery( remote_branch_exists: bool | None = None, recorded_base_sha: str | None = None, base_ancestry: Mapping[str, Any] | None = None, + sync_provenance: Mapping[str, Any] | None = None, ) -> dict[str, Any]: """Decide whether a dead-session author lock may be natively recovered. @@ -469,19 +540,39 @@ def assess_dead_session_lock_recovery( head_relation = HEAD_RELATION_STRICT_DESCENDANT ancestry_proof = notes[0] if notes else None else: - reasons.append( - f"local head {local_head} does not match remote branch head " - f"{remote_head}" + # #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, ) - reasons.extend(notes) + if synced: + head_relation = HEAD_RELATION_REMOTE_MERGE_SYNCED + ancestry_proof = sync_notes[0] if sync_notes else None + else: + reasons.append( + f"local head {local_head} does not match remote branch " + f"head {remote_head}" + ) + reasons.extend(notes) + reasons.extend(sync_notes) evidence["recorded_base"] = recorded_base or None evidence["local_head"] = local_head or None evidence["remote_head"] = remote_head or None # ``recorded_head`` is the head recovery is being measured against; # ``accepted_head`` is the head this recovery actually adopts. They differ # 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["accepted_head"] = local_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["head_relation"] = head_relation evidence["ancestry_proof"] = ancestry_proof @@ -493,12 +584,17 @@ def assess_dead_session_lock_recovery( # contradictory; re-stating it as a head mismatch would only obscure why. if not unpublished and local_head and pr_head != local_head: # A descendant recovery has not been published yet, so the open PR - # legitimately still points at the recorded head. Any other - # disagreement is a real mismatch. + # legitimately still points at the recorded head. A merge-sync + # recovery's PR legitimately sits at the advanced remote head. Any + # other disagreement is a real mismatch. if not ( head_relation == HEAD_RELATION_STRICT_DESCENDANT and 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( f"open PR #{pr_number} head {pr_head} does not match local head " @@ -619,7 +715,11 @@ def assess_dead_session_lock_recovery( ) if ( head_relation - in (HEAD_RELATION_STRICT_DESCENDANT, HEAD_RELATION_DESCENDS_FROM_BASE) + in ( + HEAD_RELATION_STRICT_DESCENDANT, + HEAD_RELATION_DESCENDS_FROM_BASE, + HEAD_RELATION_REMOTE_MERGE_SYNCED, + ) and ancestry_proof ): proof.append(ancestry_proof) @@ -697,6 +797,16 @@ def owning_pr_recovery_evidence( return None if accepted_head != local_head: 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: return None try: @@ -765,6 +875,18 @@ def recovered_owning_pr_from_lock( return None if not accepted_head or accepted_head == recorded_head: 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: return None try: diff --git a/issue_lock_store.py b/issue_lock_store.py index 713fa2a..a963174 100644 --- a/issue_lock_store.py +++ b/issue_lock_store.py @@ -737,4 +737,308 @@ def format_lock_proof( parts.append("lock released") elif released is False: parts.append("lock retained") - return "; ".join(parts) \ No newline at end of file + 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 \ No newline at end of file diff --git a/issue_lock_worktree.py b/issue_lock_worktree.py index de52ff7..8188536 100644 --- a/issue_lock_worktree.py +++ b/issue_lock_worktree.py @@ -145,6 +145,175 @@ def read_head_ancestry( 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( worktree_path: str, *, diff --git a/tests/test_issue_871_durable_lock_head_refresh.py b/tests/test_issue_871_durable_lock_head_refresh.py new file mode 100644 index 0000000..37851b1 --- /dev/null +++ b/tests/test_issue_871_durable_lock_head_refresh.py @@ -0,0 +1,630 @@ +"""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", "t@example.com") + _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", "t@example.com") + _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()