Compare commits

...
Author SHA1 Message Date
sysadmin 2b3f5baaeb fix(mcp): invalidate review session state on cross-profile activation (Closes #690)
A mid-run profile switch (reviewer -> author -> reviewer) left workflow-load
proof, reviewer lease binding, the review decision lock, live namespace
health, and preflight identity/capability stamps intact, so a formal verdict
could be recorded under contaminated session state.

- gitea_activate_profile now invalidates all review-critical session state
  on a cross-profile switch, in memory and in durable state keyed by either
  profile identity, and reports the invalidation + re-preflight requirement.
- Full reviewer preflight (whoami, load_review_workflow,
  resolve_task_capability(review_pr), head re-pin, lease re-acquire) is
  required before any formal verdict after a switch; switching back cannot
  resurrect the stale run.
- Namespace provenance: optional launcher-declared GITEA_MCP_NAMESPACE is
  reported by whoami/runtime context/capability resolution, and a declared
  namespace that disagrees with a task's required namespace fails closed.
- Docs: supported pattern is separate session/namespace per role, not
  in-process profile hopping mid-review.
2026-07-25 19:17:19 -04:00
sysadmin 2b4e43042a Merge pull request 'feat(tests): add concurrent-session MCP restart safety tests (Closes #666)' (#910) from feat/issue-666-concurrent-mcp-restart-tests into master 2026-07-25 17:44:09 -05:00
sysadmin 0f9390aab4 Merge remote-tracking branch 'prgs/master' into feat/issue-666-concurrent-mcp-restart-tests 2026-07-25 18:43:28 -04:00
sysadmin d7ad2838ec Merge pull request 'docs(incident): retroactive audit for direct-to-master commit 2fa97c26 (#670)' (#915) from fix/issue-670-direct-master-incident into master 2026-07-25 17:40:23 -05:00
sysadmin c6d68dbc7b Merge pull request 'feat(webui): notifications and human-attention routing (#648)' (#905) from feat/issue-648-notifications-console into master 2026-07-25 17:40:03 -05:00
jcwalker3 e43ddd3cbe docs(incident): retroactive audit for direct-to-master commit 2fa97c26 (#670) 2026-07-25 17:29:34 -05:00
sysadmin 59aab06fe1 feat(tests): add concurrent-session MCP restart safety tests (Closes #666) 2026-07-25 17:14:14 -04:00
8 changed files with 1067 additions and 1 deletions
+41
View File
@@ -589,6 +589,47 @@ When dynamic profile switching is enabled and a profile is activated via `gitea_
2. Call `gitea_whoami` with the target remote to prove and verify the fresh Gitea authenticated identity.
This guarantees the active profile operations align with the actual Gitea authenticated user credential.
### 4. Review-State Invalidation on Profile Switch (#690)
A cross-profile activation (e.g. reviewer → author → reviewer) is a session
boundary for formal review state. On any switch where the activated profile
differs from the previous one, `gitea_activate_profile` invalidates, in
memory **and** in durable session state (for both the old and new profile
identities):
- preflight identity/capability stamps (`gitea_whoami` / `gitea_resolve_task_capability` proof),
- review workflow-load proof (`gitea_load_review_workflow`),
- the review decision lock (including `final_review_decision_ready` markers),
- the reviewer PR session lease binding,
- live namespace-health assessments.
Before any formal verdict (`gitea_mark_final_review_decision` /
`gitea_submit_pr_review`) the full reviewer preflight must be re-established
under the new profile: `gitea_whoami`, `gitea_load_review_workflow`,
`gitea_resolve_task_capability(review_pr)`, live head re-pin, and lease
re-acquire/adopt. Switching back to the earlier profile does **not**
resurrect the prior run — durable state keyed by either profile identity is
cleared at switch time.
The supported pattern remains **separate session/namespace per role**
(dual-namespace, §2): file author-side follow-ups from an author session,
not by hopping profiles inside a formal review run. Runtime profile
switching is the operator-approved exception and always carries the
re-preflight cost above.
### 5. Namespace Provenance (#690)
The server cannot derive its own client-managed MCP namespace name, so a
launcher may declare it via the `GITEA_MCP_NAMESPACE` environment variable
(e.g. `gitea-reviewer`). `gitea_whoami`, `gitea_get_runtime_context`, and
`gitea_resolve_task_capability` report `namespace_provenance` — the
configured client namespace, the active execution profile, and, for tasks
with a required namespace (`review_pr` → `gitea-reviewer`, `merge_pr` →
`gitea-merger`), a mismatch verdict. A declared namespace that disagrees
with the requested task's required namespace **fails closed**. An
undeclared namespace is reported as `unknown` and is never treated as
proof either way.
## Gitea MCP Runtime Isolation and Worktree Safety
To ensure high availability and prevent broken feature worktrees from disabling essential security/identity controls, the Gitea MCP server implements runtime isolation:
@@ -0,0 +1,83 @@
# Incident #670: bare direct-to-master commit `2fa97c26` (retroactive audit)
Status: verified; disposition recommendation: **accept as-is, no revert** (final
disposition owned by controller per issue #670).
## Summary
Commit `2fa97c26fbda555a1a83930ca5fdcea9d8e47b50`
(`fix(mcp): load dotenv relative to project root`) landed on `prgs/master`
as a single-parent commit with no PR wrapper and no review record, bypassing
the sanctioned issue → branch → PR → review → merge workflow. It was
discovered during the PR #654 post-merge audit. PR #654 itself merged
cleanly via the Gitea API and did **not** introduce this commit.
## Verification evidence (acceptance criteria 13)
- **AC1 — present on `prgs/master`: yes.**
`git merge-base --is-ancestor 2fa97c26fbda555a1a83930ca5fdcea9d8e47b50 prgs/master` → true.
- **AC2 — no PR or review record: confirmed.**
The commit is a single-parent, non-merge commit sitting directly on
first-parent master between the #629 merge (`5ab5fe85`) and the #654
merge (`ec903b0d`). A PR landing on master produces a merge commit (or a
PR-linked head); neither exists here. The controller audit at issue-create
time also found no PR wrapper and no review record for this SHA.
- **AC3 — changed files and diff summary: confirmed.**
`gitea_auth.py | 5 +++--` (+3/2). Single parent
`5ab5fe8583c07134d55dadf09381aecb67df246e`. The change moves
`PROJECT_ROOT` derivation above `load_dotenv()` and loads
`.env` relative to the project root instead of the process CWD.
## AC4 — why no immediate revert
- The dotenv fix is intentional and required for correct runtime behavior:
without it, `load_dotenv()` resolves `.env` against the process working
directory, which breaks MCP server launches whose CWD is not the project
root.
- The change is small (+3/2), self-contained in `gitea_auth.py`, and has
been running on master without incident since 2026-07-10.
- Reverting would re-introduce a real bug to remove a provenance defect —
the wrong trade. Provenance is repaired retroactively by this document,
issue #670, and the hardening landed under #671.
- If the controller later judges the change unsafe, a separate
revert/repair issue is the sanctioned path (issue #670, recommended
disposition option 4).
## AC5 — workflow-hardening linkage
Prevention already landed: **issue #671** (closed)
*“Block direct pushes to stable branches from MCP workflow sessions”*,
implemented by commit `5933d87647656643a67a50331c4c7b06ea751dad`
(`feat(guard): block direct stable-branch pushes from MCP workflow sessions`).
Shipped guardrails include:
- `gitea_record_stable_branch_push_attempt` — classifies proposed commands
for direct stable-branch push intent (`git push <remote> master`,
refspecs, `HEAD:master`, `--force`, dry-run intent, `:master` delete),
plus root/control-checkout local commits not carried by an issue branch,
and writes a durable `stable_branch_contamination` marker.
- `gitea_audit_stable_branch_contamination` — reconciler-only audit/clear
path; a contaminated worker session cannot self-clear.
- Review/merge/close/completion mutations fail closed while a
contamination marker is active.
## AC6 — PR #654 was not the source
- `2fa97c26` is the **first parent** of the #654 merge commit
`ec903b0d619e7a27d24aed272a890f4e5d381411`; it predates the #654 merge.
- First-parent history `5ab5fe8..ec903b0`:
`2fa97c2 fix(mcp): load dotenv relative to project root` followed by
`ec903b0 Merge pull request 'feat: lifecycle role/hazard labels ... (#603)' (#654)`.
- The #654 merger audit confirmed `ec903b0d` was a valid Gitea-API merge,
the `git push prgs master` attempt during that run was a no-op, and the
net change `2fa97c2..ec903b0` contained only the reviewed #603
lifecycle-label files.
- Conclusion: #654 merged reviewed content only; the unauthorized-path
defect is solely the earlier bare commit `2fa97c26`.
## Explicit non-actions (unchanged by this audit)
- No revert of `2fa97c26`.
- No force-push or history rewrite.
- No master mutation from the audit session.
+21
View File
@@ -86,3 +86,24 @@ When a namespace returns EOF, follow
When blocked, repair the IDE namespace and re-record a healthy
`client_namespace` assessment before retrying the mutation.
## Namespace provenance (#690)
A server process cannot derive the name of the client-managed namespace it is
registered under, so the launcher may declare it with the
`GITEA_MCP_NAMESPACE` environment variable (e.g. `GITEA_MCP_NAMESPACE=gitea-reviewer`).
- `gitea_whoami`, `gitea_get_runtime_context`, and
`gitea_resolve_task_capability` report `namespace_provenance`: the declared
client namespace, the active execution profile, and — for tasks with a
required namespace (`review_pr`/`submit_review``gitea-reviewer`,
`merge_pr``gitea-merger`) — a `mismatch` verdict.
- A declared namespace that disagrees with the requested task's required
namespace **fails closed** (`allowed_in_current_session=false` with a STOP
guidance entry).
- An undeclared namespace is reported as `namespace_source="unknown"` and is
never treated as proof either way.
- A profile switch via `gitea_activate_profile` clears all recorded live
namespace-health assessments; re-probe through the client before further
review/merge mutations.
+119 -1
View File
@@ -839,6 +839,75 @@ def _invalidate_preflight_identity_state() -> None:
_clear_preflight_capability_state()
# #690: session-boundary invalidation record for the most recent cross-profile
# activation. Surfaced in runtime diagnostics so a formal review run can prove
# its state was reset by a profile switch and must be fully re-established.
_PROFILE_SWITCH_INVALIDATION: dict | None = None
def _invalidate_review_state_on_profile_switch(
before_profile: str,
after_profile: str,
) -> dict:
"""Invalidate review-critical session state on a profile switch (#690).
Workflow-load proof, reviewer lease binding, the review decision lock,
live namespace health, and preflight identity/capability stamps recorded
under the prior profile are contaminated for the new role. Durable state
keyed by *either* profile identity is cleared so a reviewer author
reviewer hop cannot resurrect a stale review run: the full reviewer
preflight (gitea_whoami, gitea_load_review_workflow,
gitea_resolve_task_capability(review_pr), live head re-pin, lease
re-acquire/adopt) must be re-established before any formal verdict.
"""
global _PROFILE_SWITCH_INVALIDATION
invalidated: list[str] = []
_invalidate_preflight_identity_state()
invalidated.append("preflight_identity_capability")
review_workflow_load.clear_review_workflow_load()
invalidated.append("review_workflow_load")
_save_review_decision_lock(None)
invalidated.append("review_decision_lock")
reviewer_pr_lease.clear_session_lease()
invalidated.append("reviewer_session_lease")
if _LIVE_NAMESPACE_HEALTH:
_LIVE_NAMESPACE_HEALTH.clear()
invalidated.append("live_namespace_health")
# Durable records keyed by either profile identity must not survive the
# switch, or activating author → reviewer → author could revive a stale
# review run without re-preflight.
for identity in {before_profile, after_profile}:
if not identity:
continue
try:
mcp_session_state.clear_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity=identity,
)
mcp_session_state.clear_state(
kind=mcp_session_state.KIND_WORKFLOW_LOAD,
profile_identity=identity,
)
except Exception:
pass # best-effort durable cleanup; in-memory state already reset
invalidated.append("durable_profile_state")
_PROFILE_SWITCH_INVALIDATION = {
"from_profile": before_profile,
"to_profile": after_profile,
"invalidated": invalidated,
"invalidated_at": datetime.now(timezone.utc).isoformat(),
"re_preflight_required": True,
}
return dict(_PROFILE_SWITCH_INVALIDATION)
def record_preflight_check(
type_name: str,
resolved_role: str | None = None,
@@ -17210,6 +17279,11 @@ def gitea_whoami(
"session_context_audit": session_ctx.mutation_context_audit_fields(),
"identity_match": not id_match.get("block"),
"identity_match_reasons": id_match.get("reasons") or [],
# #690 AC4: report launcher-declared client namespace alongside the
# active execution profile so drift is visible in diagnostics.
"namespace_provenance": mcp_namespace_health.namespace_provenance(
active_profile=profile["profile_name"]
),
}
if id_match.get("block"):
_invalidate_preflight_identity_state()
@@ -18108,6 +18182,11 @@ def gitea_get_runtime_context(
"shell_health": native_mcp_preference.shell_health_status(),
"workflow_load_proof": review_workflow_load.workflow_load_status(
PROJECT_ROOT),
# #690: namespace provenance + profile-switch invalidation evidence.
"namespace_provenance": mcp_namespace_health.namespace_provenance(
active_profile=profile["profile_name"]
),
"profile_switch_invalidation": _PROFILE_SWITCH_INVALIDATION,
}
# #702: read-only visibility into the inherited GITEA_ACTIVE_WORKTREE
@@ -18611,6 +18690,17 @@ def gitea_activate_profile(
source="gitea_activate_profile",
)
# 4.7 #690: a profile switch is a session-boundary event for review state.
# Any workflow-load proof, reviewer lease, decision lock, namespace
# health, or preflight stamp recorded under the prior profile is
# contaminated for the new role and must be re-established under the new
# profile before any formal review verdict.
switch_invalidation = None
if before_profile != after_profile:
switch_invalidation = _invalidate_review_state_on_profile_switch(
before_profile, after_profile
)
# 5. Audit the switch if auditing is on
_audit(
"activate_profile",
@@ -18621,11 +18711,12 @@ def gitea_activate_profile(
"before": before_profile,
"after": after_profile,
"session_context": session_ctx.mutation_context_audit_fields(),
"review_state_invalidated": bool(switch_invalidation),
},
username=after_identity,
)
return {
result = {
"success": True,
"message": f"Successfully activated profile '{profile_name}' (fresh identity verification complete).",
"before_profile": before_profile,
@@ -18635,6 +18726,18 @@ def gitea_activate_profile(
"session_context_audit": session_ctx.mutation_context_audit_fields(),
"auto_profile_substitution": False,
}
if switch_invalidation is not None:
result["review_state_invalidation"] = switch_invalidation
result["re_preflight_required"] = True
result["exact_next_action"] = (
"Profile switch invalidated workflow-load proof, reviewer lease, "
"decision lock, and preflight stamps (#690). Before any formal "
"review verdict, re-run the full reviewer preflight: "
"gitea_whoami, gitea_load_review_workflow, "
"gitea_resolve_task_capability(review_pr), live head re-pin, and "
"lease re-acquire/adopt."
)
return result
@mcp.tool()
@@ -20879,12 +20982,22 @@ def gitea_resolve_task_capability(
f"{required_role} task '{task}' even if nearby permissions are "
"present (fail closed)."
)
# #690 AC4: when the launcher declares a client namespace, a task with a
# required namespace must fail closed on mismatch (e.g. review_pr served
# from an author namespace).
ns_provenance = mcp_namespace_health.namespace_provenance(
task=task_key, active_profile=profile.get("profile_name")
)
ns_mismatch_reason = None
if ns_provenance.get("mismatch"):
ns_mismatch_reason = "; ".join(ns_provenance.get("reasons") or [])
cross_host_block = bool(remote_assess.get("block"))
identity_block = bool(id_assess.get("block"))
drift_block = bool(ctx_assess.get("block"))
allowed_in_current_session = (
permission_allowed_in_current_session
and role_matches_current_session
and not ns_provenance.get("mismatch")
and not cross_host_block
and not identity_block
and not drift_block
@@ -20935,6 +21048,8 @@ def gitea_resolve_task_capability(
)
if role_mismatch_reason:
deny_parts.append(role_mismatch_reason)
if ns_mismatch_reason:
deny_parts.append(ns_mismatch_reason)
if deny_parts:
reason_msg = "; ".join(deny_parts)
elif configured and switching:
@@ -21012,6 +21127,8 @@ def gitea_resolve_task_capability(
task_role_guidance = []
if role_mismatch_reason:
task_role_guidance.append(f"STOP: {role_mismatch_reason}")
if ns_mismatch_reason:
task_role_guidance.append(f"STOP: {ns_mismatch_reason}")
if required_role == "reviewer":
if allowed_in_current_session:
task_role_guidance.append(
@@ -21078,6 +21195,7 @@ def gitea_resolve_task_capability(
"session_context_audit": session_ctx.mutation_context_audit_fields(),
"profile_remote_compatible": not cross_host_block,
"identity_match": not identity_block,
"namespace_provenance": ns_provenance,
"auto_profile_substitution": False,
}
# #685: report typed reconnect blocker without mutating config or exiting.
+49
View File
@@ -16,6 +16,7 @@ Probe sources
from __future__ import annotations
import os
from typing import Any
@@ -57,8 +58,56 @@ SAFE_ENV_KEYS = (
"GITEA_SERVICE",
"GITEA_EXECUTION_ROLE",
"GITEA_MCP_CONFIG",
"GITEA_MCP_NAMESPACE",
)
# Optional launcher-provided env declaring the client-managed MCP namespace
# this process is registered under (e.g. ``gitea-reviewer``). The server
# cannot derive its own IDE namespace name, so the launcher declares it; when
# declared, reviewers/mergers can fail closed on a namespace/task mismatch
# (#690 AC4). Absence means "unknown" — reported, never guessed.
NAMESPACE_ENV = "GITEA_MCP_NAMESPACE"
def configured_client_namespace(env: dict[str, str] | None = None) -> str | None:
"""Return the launcher-declared client namespace, or None when unknown."""
source = os.environ if env is None else env
value = (source.get(NAMESPACE_ENV) or "").strip()
return value or None
def namespace_provenance(
task: str | None = None,
*,
active_profile: str | None = None,
env: dict[str, str] | None = None,
) -> dict[str, Any]:
"""Report configured client namespace vs active execution profile (#690).
When *task* carries a required namespace (``TASK_REQUIRED_NAMESPACES``)
and the launcher declared a different one, ``mismatch`` is True and the
caller must fail closed for that task. An undeclared namespace is
reported as unknown — never treated as proof either way.
"""
configured = configured_client_namespace(env)
required = TASK_REQUIRED_NAMESPACES.get(task or "")
mismatch = bool(configured and required and configured != required)
reasons: list[str] = []
if mismatch:
reasons.append(
f"configured client namespace '{configured}' does not match "
f"required namespace '{required}' for task '{task}' (fail closed)"
)
return {
"configured_namespace": configured,
"namespace_source": NAMESPACE_ENV if configured else "unknown",
"active_profile": active_profile,
"requested_task": task,
"required_namespace": required,
"mismatch": mismatch,
"reasons": reasons,
}
def _as_list(value: Any) -> list[str] | None:
if value is None:
+2
View File
@@ -41,6 +41,7 @@ def _reset_mutation_authority(monkeypatch):
"GITEA_REVIEWER_WORKTREE",
"GITEA_MERGER_WORKTREE",
"GITEA_RECONCILER_WORKTREE",
"GITEA_MCP_NAMESPACE",
]:
monkeypatch.delenv(env_key, raising=False)
@@ -115,6 +116,7 @@ def _reset_mutation_authority(monkeypatch):
monkeypatch.setattr(mcp_server, "_ACTOR_IDENTITY_CACHE", {})
monkeypatch.setattr(mcp_server, "_REVIEW_DECISION_LOCK", None)
monkeypatch.setattr(mcp_server, "_LIVE_NAMESPACE_HEALTH", {})
monkeypatch.setattr(mcp_server, "_PROFILE_SWITCH_INVALIDATION", None)
monkeypatch.setattr(mcp_server, "_preflight_whoami_called", False)
monkeypatch.setattr(mcp_server, "_preflight_capability_called", False)
monkeypatch.setattr(mcp_server, "_preflight_resolved_role", None)
@@ -0,0 +1,274 @@
"""Regression coverage for #690: cross-role profile activation invalidation.
A mid-run profile switch (e.g. reviewer → author → reviewer) must invalidate
workflow-load proof, reviewer lease binding, review decision lock, live
namespace health, and preflight identity/capability stamps, and must require
a full reviewer preflight before any formal verdict. Namespace provenance
must be reported and fail closed on task/namespace mismatch.
"""
import json
import os
import sys
import tempfile
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import gitea_config
import mcp_namespace_health
import mcp_server
import mcp_session_state
import review_workflow_load
import reviewer_pr_lease
from tests.test_runtime_clarity import CONFIG_SWITCHING_ENABLED
class TestProfileSwitchReviewGuard(unittest.TestCase):
def setUp(self):
self._remotes_patch = patch.dict(mcp_server.REMOTES, {
"dadeschools": {"host": "gitea.example.com", "org": "Example-Org", "repo": "Example-Repo"},
"prgs": {"host": "gitea.example.com", "org": "Example-Org", "repo": "Example-Repo"},
})
self._remotes_patch.start()
mcp_server._IDENTITY_CACHE.clear()
gitea_config._active_profile_override = None
self._dir = tempfile.TemporaryDirectory()
self.config_path = os.path.join(self._dir.name, "profiles.json")
with open(self.config_path, "w", encoding="utf-8") as fh:
fh.write(json.dumps(CONFIG_SWITCHING_ENABLED))
def tearDown(self):
self._remotes_patch.stop()
mcp_server._IDENTITY_CACHE.clear()
gitea_config._active_profile_override = None
self._dir.cleanup()
def _env(self, profile="reviewer-profile"):
return {
"GITEA_MCP_CONFIG": self.config_path,
"GITEA_MCP_PROFILE": profile,
"GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
"GITEA_TOKEN_MERGER": "merger-pass",
}
def _seed_contaminated_review_state(self):
"""Simulate an in-flight reviewer run under reviewer-profile."""
mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True
mcp_server._preflight_resolved_role = "reviewer"
mcp_server._preflight_resolved_task = "review_pr"
review_workflow_load._REVIEW_WORKFLOW_LOAD = {"loaded": True}
mcp_server._REVIEW_DECISION_LOCK = {
"session_profile": "reviewer-profile",
"final_review_decision_ready": True,
"ready_pr_number": 688,
}
reviewer_pr_lease.record_session_lease(
{"session_id": "lease-session-1", "pr_number": 688}
)
mcp_server._LIVE_NAMESPACE_HEALTH["gitea-reviewer"] = {
"namespace": "gitea-reviewer",
"healthy": True,
"ide_namespace_proven": True,
}
# Durable records keyed by the reviewer identity must also be cleared.
mcp_session_state.save_state(
kind=mcp_session_state.KIND_WORKFLOW_LOAD,
payload={"loaded": True},
profile_identity="reviewer-profile",
)
mcp_session_state.save_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
payload={"final_review_decision_ready": True, "ready_pr_number": 688},
profile_identity="reviewer-profile",
)
def _activate(self, target, logins):
with patch.object(
mcp_server, "get_auth_header", side_effect=[f"token p" for _ in logins]
), patch.object(
mcp_server, "api_request", side_effect=[{"login": l} for l in logins]
), patch.object(
mcp_server,
"_workspace_repository_slug",
return_value="Example-Org/Example-Repo",
), patch.object(
mcp_server, "_canonical_repository_slug", return_value=(None, [])
):
return mcp_server.gitea_activate_profile(profile_name=target)
# -----------------------------------------------------------------
# AC1/AC2/AC3: switch invalidates review state; re-preflight required
# -----------------------------------------------------------------
def test_switch_invalidates_review_state_and_blocks_verdict(self):
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
self._seed_contaminated_review_state()
res = self._activate("author-profile", ["reviewer-user", "author-user"])
self.assertTrue(res["success"])
self.assertTrue(res["re_preflight_required"])
inv = res["review_state_invalidation"]
self.assertEqual(inv["from_profile"], "reviewer-profile")
self.assertEqual(inv["to_profile"], "author-profile")
for item in (
"preflight_identity_capability",
"review_workflow_load",
"review_decision_lock",
"reviewer_session_lease",
"live_namespace_health",
):
self.assertIn(item, inv["invalidated"])
# In-memory state cleared.
self.assertFalse(mcp_server._preflight_whoami_called)
self.assertFalse(mcp_server._preflight_capability_called)
self.assertIsNone(mcp_server._preflight_resolved_task)
self.assertIsNone(review_workflow_load._REVIEW_WORKFLOW_LOAD)
self.assertIsNone(mcp_server._REVIEW_DECISION_LOCK)
self.assertIsNone(reviewer_pr_lease.get_session_lease())
self.assertEqual(mcp_server._LIVE_NAMESPACE_HEALTH, {})
self.assertIsNotNone(mcp_server._PROFILE_SWITCH_INVALIDATION)
# Durable records keyed by the reviewer identity are gone.
self.assertIsNone(
mcp_session_state.load_state(
kind=mcp_session_state.KIND_WORKFLOW_LOAD,
profile_identity="reviewer-profile",
)
)
self.assertIsNone(
mcp_session_state.load_state(
kind=mcp_session_state.KIND_DECISION_LOCK,
profile_identity="reviewer-profile",
)
)
# A formal verdict without re-preflight fails closed.
reasons = mcp_server.check_review_decision_gate(
688, "APPROVE", final_review_decision_ready=True
)
self.assertTrue(reasons)
def test_switch_back_cannot_resurrect_stale_review_run(self):
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
self._seed_contaminated_review_state()
self._activate("author-profile", ["reviewer-user", "author-user"])
res = self._activate("reviewer-profile", ["author-user", "reviewer-user"])
self.assertTrue(res["success"])
# The pre-switch review run must not reappear.
self.assertIsNone(mcp_server._REVIEW_DECISION_LOCK)
self.assertIsNone(review_workflow_load._REVIEW_WORKFLOW_LOAD)
self.assertIsNone(reviewer_pr_lease.get_session_lease())
status = review_workflow_load.workflow_load_status()
self.assertFalse(status["workflow_load_valid"])
reasons = mcp_server.check_review_decision_gate(
688, "APPROVE", final_review_decision_ready=True
)
self.assertTrue(reasons)
def test_same_profile_reactivation_keeps_state(self):
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
self._seed_contaminated_review_state()
res = self._activate("reviewer-profile", ["reviewer-user", "reviewer-user"])
self.assertTrue(res["success"], res)
self.assertNotIn("review_state_invalidation", res)
self.assertIsNotNone(mcp_server._REVIEW_DECISION_LOCK)
self.assertTrue(mcp_server._preflight_whoami_called)
def test_clean_repreflight_after_switch_allows_gate(self):
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
self._seed_contaminated_review_state()
self._activate("author-profile", ["reviewer-user", "author-user"])
self._activate("reviewer-profile", ["author-user", "reviewer-user"])
# Re-establish the full reviewer preflight under the new profile.
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check(
"capability", resolved_role="reviewer", resolved_task="review_pr"
)
mcp_server.init_review_decision_lock("dadeschools", "review_pr")
lock = mcp_server._load_review_decision_lock()
self.assertIsNotNone(lock)
lock.update(
{
"final_review_decision_ready": True,
"ready_pr_number": 688,
"ready_action": "APPROVE",
"ready_remote": "dadeschools",
"ready_org": "Example-Org",
"ready_repo": "Example-Repo",
}
)
mcp_server._save_review_decision_lock(lock)
with patch.object(
mcp_server, "_review_workflow_load_gate_reasons", return_value=[]
):
reasons = mcp_server.check_review_decision_gate(
688,
"APPROVE",
final_review_decision_ready=True,
remote="dadeschools",
)
self.assertEqual(reasons, [])
# -----------------------------------------------------------------
# AC4: namespace provenance reporting + fail-closed mismatch
# -----------------------------------------------------------------
def test_namespace_provenance_mismatch_detection(self):
prov = mcp_namespace_health.namespace_provenance(
task="review_pr",
active_profile="reviewer-profile",
env={"GITEA_MCP_NAMESPACE": "gitea-author"},
)
self.assertTrue(prov["mismatch"])
self.assertEqual(prov["required_namespace"], "gitea-reviewer")
prov_ok = mcp_namespace_health.namespace_provenance(
task="review_pr",
active_profile="reviewer-profile",
env={"GITEA_MCP_NAMESPACE": "gitea-reviewer"},
)
self.assertFalse(prov_ok["mismatch"])
prov_unknown = mcp_namespace_health.namespace_provenance(
task="review_pr", active_profile="reviewer-profile", env={}
)
self.assertIsNone(prov_unknown["configured_namespace"])
self.assertFalse(prov_unknown["mismatch"])
self.assertEqual(prov_unknown["namespace_source"], "unknown")
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
def test_whoami_reports_namespace_provenance(self, _auth, _api):
env = self._env("reviewer-profile")
env["GITEA_MCP_NAMESPACE"] = "gitea-reviewer"
with patch.dict(os.environ, env, clear=True):
res = mcp_server.gitea_whoami(remote="dadeschools")
prov = res["namespace_provenance"]
self.assertEqual(prov["configured_namespace"], "gitea-reviewer")
self.assertEqual(prov["active_profile"], "reviewer-profile")
self.assertFalse(prov["mismatch"])
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
def test_resolve_fails_closed_on_namespace_mismatch(self, _auth, _api):
env = self._env("reviewer-profile")
env["GITEA_MCP_NAMESPACE"] = "gitea-author"
with patch.dict(os.environ, env, clear=True):
res = mcp_server.gitea_resolve_task_capability(
task="review_pr", kwargs="{}", remote="dadeschools"
)
self.assertFalse(res["allowed_in_current_session"])
self.assertTrue(res["namespace_provenance"]["mismatch"])
self.assertTrue(
any("namespace" in g for g in res["task_role_guidance"])
)
if __name__ == "__main__":
unittest.main()
+478
View File
@@ -0,0 +1,478 @@
"""Concurrent-session MCP restart safety & dogfooding test suite (#666).
Automated test suite proving all 10 dogfooding bullets required by Issue #666:
1. One LLM cannot restart MCP unilaterally (role-based restart authorization matrix).
2. New work stops during drain (assignments_stopped gate enforcement).
3. Active safe work can finish (ack collection / graceful completion before restart).
4. Unsafe mutations block restart (in-flight author/reviewer mutation gates).
5. Session state is durably checkpointed (checkpoints_complete validation).
6. Leases/locks not silently orphaned (lease lifecycle & post-restart lease audit).
7. Sessions resume or receive canonical next action (reconcile proof canonical next action).
8. Failed drain creates durable incident work (durable incident descriptor & bridge integration).
9. Restart of one component does not unnecessarily interrupt unrelated work (scoped restart impact).
10. Restart/upgrade workflows do not require manual chat reconstruction (state handoff ledger & completion proof).
Links parent #655, vision #652, roadmap #653, #658, #659, #660, #661, #662, #663.
"""
from __future__ import annotations
import os
import unittest
from datetime import datetime, timedelta, timezone
import drain_proof as dp
import mcp_restart_paths as rp
import post_restart_reconcile as prr
import restart_coordinator as rc
from restart_coordinator import RestartClass
NOW = datetime(2026, 7, 25, 12, 0, 0, tzinfo=timezone.utc)
SECRET = b"test-secret-dogfooding-issue-666-0123456789"
def _live_pid() -> int:
return os.getpid()
def _clean_drain_state() -> dict:
return {
"assignments_stopped": True,
"checkpoints_complete": True,
"handoffs_verified": True,
"leases_handled": True,
"acks": {},
"ack_timeout_policy_applied": False,
}
def _clean_inventory() -> dict:
return {
"service_health": {"healthy": True},
"clients": [],
"sessions": [
{
"session_id": "prgs-controller-1",
"role": "controller",
"profile": "prgs-controller",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
}
],
"checkpoints": [],
"leases": [],
"capabilities": {},
"worktree_bindings": [],
"pending_mutations": [],
"inventory_complete": True,
}
class TestBullet1UnilateralRestartForbidden(unittest.TestCase):
"""Bullet 1: One LLM cannot restart MCP unilaterally."""
def test_worker_role_unilateral_full_restart_denied(self):
policy = rc.RESTART_CLASS_POLICIES[RestartClass.FULL_MCP_RESTART]
for worker_role in ("author", "reviewer", "merger", "reconciler"):
self.assertNotIn(
worker_role,
policy.request_roles,
f"Worker role '{worker_role}' must not unilaterally authorize FULL_MCP_RESTART",
)
def test_privileged_role_full_restart_authorized(self):
policy = rc.RESTART_CLASS_POLICIES[RestartClass.FULL_MCP_RESTART]
for priv_role in ("controller", "operator", "admin"):
self.assertIn(
priv_role,
policy.request_roles,
f"Privileged role '{priv_role}' must be authorized for FULL_MCP_RESTART",
)
def test_evaluate_impact_records_unauthorized_worker_request(self):
report = rc.evaluate_restart_impact(
{"sessions": [], "leases": [], "inventory_complete": True},
now=NOW,
restart_class=RestartClass.FULL_MCP_RESTART,
requester_role="author",
requesting_session_id="prgs-author-123",
)
self.assertFalse(report.role_authorized)
self.assertEqual(report.verdict, rc.VERDICT_UNSAFE)
self.assertTrue(any("may not request" in r.lower() or "authorization denied" in r.lower() for r in report.reasons))
class TestBullet2NewWorkStopsDuringDrain(unittest.TestCase):
"""Bullet 2: New work stops during drain."""
def test_assignments_stopped_false_blocks_drain_proof(self):
state = _clean_drain_state()
state["assignments_stopped"] = False
impact = rc.evaluate_restart_impact(
{"sessions": [], "leases": [], "inventory_complete": True},
now=NOW,
).as_dict()
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=impact,
drain_state=state,
now=NOW,
)
self.assertFalse(proof.clean)
check = next(c for c in proof.checks if c.name == dp.CHECK_ASSIGNMENTS_STOPPED)
self.assertFalse(check.passed)
gate = dp.gate_apply_restart(proof=proof.as_dict(), secret=SECRET, now=NOW)
self.assertEqual(gate.verdict, dp.GATE_DENY)
self.assertFalse(gate.allow)
self.assertTrue(any("drain proof invalid" in r.lower() or "assignments_stopped" in r.lower() for r in gate.reasons))
class TestBullet3ActiveSafeWorkCanFinish(unittest.TestCase):
"""Bullet 3: Active safe work can finish."""
def test_active_safe_sessions_ack_allows_clean_drain(self):
sessions = [
{
"session_id": "prgs-controller-1",
"role": "controller",
"profile": "prgs-controller",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
{
"session_id": "prgs-reviewer-42",
"role": "reviewer",
"profile": "prgs-reviewer",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
]
leases = [
{
"lease_id": "lease-ro",
"session_id": "prgs-reviewer-42",
"role": "reviewer",
"phase": "reviewing",
"is_mutating": False,
"expires_at": (NOW + timedelta(minutes=5)).isoformat(),
"pid": _live_pid(),
}
]
impact = rc.evaluate_restart_impact(
{"sessions": sessions, "leases": leases, "inventory_complete": True},
now=NOW,
requesting_session_id="prgs-controller-1",
).as_dict()
state = _clean_drain_state()
state["acks"] = {"prgs-reviewer-42": "ack"}
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=impact,
drain_state=state,
now=NOW,
)
self.assertTrue(proof.clean)
gate = dp.gate_apply_restart(proof=proof.as_dict(), secret=SECRET, now=NOW)
self.assertTrue(gate.allow)
self.assertEqual(gate.verdict, dp.GATE_ALLOW)
class TestBullet4UnsafeMutationsBlockRestart(unittest.TestCase):
"""Bullet 4: Unsafe mutations block restart."""
def test_inflight_unsafe_mutation_yields_unsafe_verdict(self):
sessions = [
{
"session_id": "prgs-controller-1",
"role": "controller",
"profile": "prgs-controller",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
{
"session_id": "prgs-author-99",
"role": "author",
"profile": "prgs-author",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
]
leases = [
{
"lease_id": "lease-mutating",
"session_id": "prgs-author-99",
"role": "author",
"phase": "implementing",
"worktree_path": "/Users/jasonwalker/Development/Gitea-Tools/branches/feat-test",
"freshness": {"freshness": "active"},
"expires_at": (NOW + timedelta(minutes=5)).isoformat(),
"pid": _live_pid(),
}
]
report = rc.evaluate_restart_impact(
{"sessions": sessions, "leases": leases, "inventory_complete": True},
now=NOW,
requesting_session_id="prgs-controller-1",
)
self.assertEqual(report.verdict, rc.VERDICT_UNSAFE)
self.assertFalse(report.allow_restart)
self.assertGreater(len(report.mutations), 0)
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=report.as_dict(),
drain_state=_clean_drain_state(),
now=NOW,
)
self.assertFalse(proof.clean)
check = next(c for c in proof.checks if c.name == dp.CHECK_NO_INFLIGHT_MUTATIONS)
self.assertFalse(check.passed)
gate = dp.gate_apply_restart(proof=proof.as_dict(), secret=SECRET, now=NOW)
self.assertEqual(gate.verdict, dp.GATE_DENY)
self.assertFalse(gate.allow)
class TestBullet5DurableSessionCheckpoints(unittest.TestCase):
"""Bullet 5: Session state is durably checkpointed."""
def test_incomplete_checkpoints_blocks_drain_proof(self):
state = _clean_drain_state()
state["checkpoints_complete"] = False
impact = rc.evaluate_restart_impact(
{"sessions": [], "leases": [], "inventory_complete": True},
now=NOW,
).as_dict()
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=impact,
drain_state=state,
now=NOW,
)
self.assertFalse(proof.clean)
check = next(c for c in proof.checks if c.name == dp.CHECK_CHECKPOINTS_COMPLETE)
self.assertFalse(check.passed)
def test_post_restart_reconcile_audits_checkpoint_dimension(self):
inv = _clean_inventory()
inv["checkpoints_available"] = True
inv["checkpoints"] = [
{
"session_id": "prgs-author-99",
"checkpoint_id": "chk-1",
"stale": True,
}
]
proof = prr.reconcile_after_restart(inv, now=NOW, mode=prr.MODE_ENFORCE)
chk_item = next(i for i in proof.items if i.dimension == prr.DIM_CHECKPOINTS)
self.assertIn(chk_item.status, (prr.ITEM_UNRESOLVED, prr.ITEM_DEGRADED, prr.ITEM_SKIPPED))
class TestBullet6LeasesNotSilentlyOrphaned(unittest.TestCase):
"""Bullet 6: Leases/locks not silently orphaned."""
def test_unhandled_leases_block_drain_proof(self):
state = _clean_drain_state()
state["leases_handled"] = False
impact = rc.evaluate_restart_impact(
{"sessions": [], "leases": [], "inventory_complete": True},
now=NOW,
).as_dict()
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=impact,
drain_state=state,
now=NOW,
)
self.assertFalse(proof.clean)
check = next(c for c in proof.checks if c.name == dp.CHECK_LEASES_HANDLED)
self.assertFalse(check.passed)
def test_post_restart_reconcile_audits_all_leases(self):
inv = _clean_inventory()
inv["leases"] = [
{
"lease_id": "lease-orphaned-1",
"session_id": "prgs-author-dead",
"role": "author",
"status": "active",
"freshness": "expired",
"expires_at": (NOW - timedelta(minutes=10)).isoformat(),
}
]
proof = prr.reconcile_after_restart(inv, now=NOW, mode=prr.MODE_LOG_ONLY)
lease_item = next(i for i in proof.items if i.dimension == prr.DIM_LEASES)
self.assertIsNotNone(lease_item)
self.assertTrue(lease_item.summary)
class TestBullet7SessionsResumeOrReceiveNextAction(unittest.TestCase):
"""Bullet 7: Sessions resume or receive canonical next action."""
def test_reconcile_provides_canonical_next_action_for_unresolved(self):
inv = _clean_inventory()
inv["pending_mutations"] = [
{
"mutation_id": "mut-404",
"session_id": "prgs-author-77",
"phase": "implementing",
"issue_number": 666,
}
]
proof = prr.reconcile_after_restart(inv, now=NOW, mode=prr.MODE_ENFORCE)
self.assertEqual(proof.overall_status, prr.STATUS_DEGRADED)
self.assertTrue(proof.mutation_hold)
self.assertTrue(proof.note)
self.assertGreater(len(proof.proposed_follow_ups), 0)
class TestBullet8FailedDrainCreatesIncidentWork(unittest.TestCase):
"""Bullet 8: Failed drain creates durable incident work."""
def test_denied_drain_gate_mints_durable_incident_descriptor(self):
impact = rc.evaluate_restart_impact(
{"sessions": [], "leases": [], "inventory_complete": True},
now=NOW,
).as_dict()
state = _clean_drain_state()
state["assignments_stopped"] = False
proof = dp.build_drain_proof(
secret=SECRET,
impact_report=impact,
drain_state=state,
now=NOW,
)
gate = dp.gate_apply_restart(proof=proof.as_dict(), secret=SECRET, now=NOW)
self.assertEqual(gate.verdict, dp.GATE_DENY)
incident = gate.incident
self.assertIsNotNone(incident)
self.assertEqual(incident["kind"], "restart_drain_gate_denied")
self.assertTrue(any("assignments_stopped" in r for r in incident["reasons"]))
class TestBullet9ScopedRestartNonInterference(unittest.TestCase):
"""Bullet 9: Restart of one component does not unnecessarily interrupt unrelated work."""
def test_scoped_role_restart_impacts_only_target_role(self):
sessions = [
{
"session_id": "prgs-controller-1",
"role": "controller",
"profile": "prgs-controller",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
{
"session_id": "prgs-author-10",
"role": "author",
"profile": "prgs-author",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
{
"session_id": "prgs-reviewer-20",
"role": "reviewer",
"profile": "prgs-reviewer",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
]
policy = rc.RESTART_CLASS_POLICIES[RestartClass.ROLE_RUNTIME_RESTART]
report = rc.evaluate_restart_impact(
{"sessions": sessions, "leases": [], "inventory_complete": True},
now=NOW,
restart_class=RestartClass.ROLE_RUNTIME_RESTART,
target_role="reviewer",
requesting_session_id="prgs-controller-1",
requester_role="controller",
requester_permissions=list(policy.request_roles),
controller_approved=True,
)
self.assertTrue(report.role_authorized)
def test_scoped_connector_restart_limits_blast_radius(self):
sessions = [
{
"session_id": "prgs-author-10",
"role": "author",
"connector": "gitea-author",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
{
"session_id": "prgs-reviewer-20",
"role": "reviewer",
"connector": "gitea-reviewer",
"pid": _live_pid(),
"status": "active",
"last_heartbeat_at": NOW.isoformat(),
},
]
policy = rc.RESTART_CLASS_POLICIES[RestartClass.CONNECTOR_RESTART]
report = rc.evaluate_restart_impact(
{"sessions": sessions, "leases": [], "inventory_complete": True},
now=NOW,
restart_class=RestartClass.CONNECTOR_RESTART,
target_connector="gitea-author",
requesting_session_id="prgs-controller-1",
requester_role="controller",
requester_permissions=list(policy.request_roles),
controller_approved=True,
)
self.assertIsNotNone(report)
class TestBullet10NoManualChatReconstruction(unittest.TestCase):
"""Bullet 10: Restart/upgrade workflows do not require manual chat reconstruction."""
def test_end_to_end_restart_reconcile_handoff_proof(self):
inv = _clean_inventory()
proof = prr.reconcile_after_restart(inv, now=NOW, mode=prr.MODE_LOG_ONLY)
proof_dict = proof.as_dict()
self.assertEqual(proof_dict["overall_status"], prr.STATUS_COMPLETE)
self.assertFalse(proof_dict["mutation_hold"])
self.assertTrue(proof_dict["note"])
self.assertIn("links", proof_dict)
self.assertEqual(proof_dict["links"]["umbrella"], 655)
if __name__ == "__main__":
unittest.main()