feat: enforce live-namespace block in review/merge state machine (#543 AC5)
The namespace health check (05fdcee) classified a false-ready namespace but
only returned an advisory blocks_merge_workflow flag; nothing in the merge
gate consumed it. Wire it in so a broken live namespace hard-blocks merge.
- review_merge_state_machine.assess_workflow_blockers: add live_namespace_broken
blocker (registered-in-FastMCP but not callable-through-namespace).
- assess_state_advancement: forward **blocker_kwargs so can_approve/can_merge/
workflow_status honor the full blocker set (also fixes latent drop of
mcp_reconnect_failed/stale_capability_state through those paths).
- gitea_assess_review_merge_state_machine tool: accept live_namespace_broken and
thread it through all state-machine calls.
- Tests: prove can_merge/workflow_status/tool block on live_namespace_broken even
with every review state + pre-merge gate satisfied; bridge classify verdict.
- Docs: enforcement section wiring blocks_merge_workflow -> live_namespace_broken.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -52,3 +52,25 @@ The read-only `gitea_assess_mcp_namespace_health` tool can classify probe
|
|||||||
evidence supplied by a client. It intentionally distinguishes
|
evidence supplied by a client. It intentionally distinguishes
|
||||||
`required_tool_registered=true` from `required_tool_callable=false`; the latter
|
`required_tool_registered=true` from `required_tool_callable=false`; the latter
|
||||||
must block reviewer and merger workflows until the live namespace is repaired.
|
must block reviewer and merger workflows until the live namespace is repaired.
|
||||||
|
|
||||||
|
## Enforcement in the review/merge state machine
|
||||||
|
|
||||||
|
The block is not advisory. Feed the `blocks_merge_workflow` verdict from
|
||||||
|
`gitea_assess_mcp_namespace_health` into
|
||||||
|
`gitea_assess_review_merge_state_machine` as `live_namespace_broken`:
|
||||||
|
|
||||||
|
```text
|
||||||
|
health = gitea_assess_mcp_namespace_health(namespace="gitea-merger", ...)
|
||||||
|
state = gitea_assess_review_merge_state_machine(
|
||||||
|
state_completion=...,
|
||||||
|
pre_merge_gates=...,
|
||||||
|
live_namespace_broken=health["blocks_merge_workflow"],
|
||||||
|
)
|
||||||
|
# state["merge"]["allowed"] is False whenever the live namespace is broken,
|
||||||
|
# even when every review state and pre-merge gate is otherwise satisfied.
|
||||||
|
```
|
||||||
|
|
||||||
|
When `live_namespace_broken=True`, `assess_workflow_blockers`,
|
||||||
|
`can_approve`, `can_merge`, and `workflow_status` all fail closed. This is the
|
||||||
|
guard that stops a merge from proceeding on a false-ready namespace, as in the
|
||||||
|
PR #418 halt that motivated this work (#543).
|
||||||
|
|||||||
+18
-13
@@ -7792,40 +7792,45 @@ def gitea_assess_review_merge_state_machine(
|
|||||||
pre_merge_gates: dict[str, bool] | None = None,
|
pre_merge_gates: dict[str, bool] | None = None,
|
||||||
infra_stop: bool = False,
|
infra_stop: bool = False,
|
||||||
capability_blocked: bool = False,
|
capability_blocked: bool = False,
|
||||||
|
live_namespace_broken: bool = False,
|
||||||
recovery_handoff_text: str | None = None,
|
recovery_handoff_text: str | None = None,
|
||||||
final_report_text: str | None = None,
|
final_report_text: str | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Read-only: assess enforced PR review/merge workflow state (#290)."""
|
"""Read-only: assess enforced PR review/merge workflow state (#290).
|
||||||
|
|
||||||
|
``live_namespace_broken`` fails review/merge closed when the live MCP
|
||||||
|
namespace call path is unusable even though the tool is registered in
|
||||||
|
FastMCP (#543 AC5). Supply the ``blocks_merge_workflow`` verdict from
|
||||||
|
``gitea_assess_mcp_namespace_health``.
|
||||||
|
"""
|
||||||
completion = state_completion or {}
|
completion = state_completion or {}
|
||||||
blockers = review_merge_state_machine.assess_workflow_blockers(
|
blocker_kwargs = {
|
||||||
infra_stop=infra_stop,
|
"infra_stop": infra_stop,
|
||||||
capability_blocked=capability_blocked,
|
"capability_blocked": capability_blocked,
|
||||||
)
|
"live_namespace_broken": live_namespace_broken,
|
||||||
|
}
|
||||||
|
blockers = review_merge_state_machine.assess_workflow_blockers(**blocker_kwargs)
|
||||||
result = {
|
result = {
|
||||||
"workflow": review_merge_state_machine.workflow_status(
|
"workflow": review_merge_state_machine.workflow_status(
|
||||||
completion,
|
completion,
|
||||||
infra_stop=infra_stop,
|
**blocker_kwargs,
|
||||||
capability_blocked=capability_blocked,
|
|
||||||
),
|
),
|
||||||
"blockers": blockers,
|
"blockers": blockers,
|
||||||
"approve": review_merge_state_machine.can_approve(
|
"approve": review_merge_state_machine.can_approve(
|
||||||
completion,
|
completion,
|
||||||
infra_stop=infra_stop,
|
**blocker_kwargs,
|
||||||
capability_blocked=capability_blocked,
|
|
||||||
),
|
),
|
||||||
"merge": review_merge_state_machine.can_merge(
|
"merge": review_merge_state_machine.can_merge(
|
||||||
completion,
|
completion,
|
||||||
pre_merge_gates=pre_merge_gates,
|
pre_merge_gates=pre_merge_gates,
|
||||||
infra_stop=infra_stop,
|
**blocker_kwargs,
|
||||||
capability_blocked=capability_blocked,
|
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
if target_state:
|
if target_state:
|
||||||
result["advancement"] = review_merge_state_machine.assess_state_advancement(
|
result["advancement"] = review_merge_state_machine.assess_state_advancement(
|
||||||
completion,
|
completion,
|
||||||
target_state=target_state,
|
target_state=target_state,
|
||||||
infra_stop=infra_stop,
|
**blocker_kwargs,
|
||||||
capability_blocked=capability_blocked,
|
|
||||||
)
|
)
|
||||||
if recovery_handoff_text is not None:
|
if recovery_handoff_text is not None:
|
||||||
result["recovery_handoff"] = (
|
result["recovery_handoff"] = (
|
||||||
|
|||||||
@@ -93,8 +93,16 @@ def assess_workflow_blockers(
|
|||||||
capability_blocked: bool = False,
|
capability_blocked: bool = False,
|
||||||
mcp_reconnect_failed: bool = False,
|
mcp_reconnect_failed: bool = False,
|
||||||
stale_capability_state: bool = False,
|
stale_capability_state: bool = False,
|
||||||
|
live_namespace_broken: bool = False,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Return hard blockers that forbid all PR queue work (#290 AC3)."""
|
"""Return hard blockers that forbid all PR queue work (#290 AC3).
|
||||||
|
|
||||||
|
``live_namespace_broken`` fails the merge/review path closed when the live
|
||||||
|
MCP namespace call path is unusable (e.g. ``client is closing: EOF``) even
|
||||||
|
though the tool is registered in FastMCP (#543 AC5). Feed it the
|
||||||
|
``blocks_merge_workflow`` verdict from
|
||||||
|
``mcp_namespace_health.classify_namespace_probe``.
|
||||||
|
"""
|
||||||
reasons: list[str] = []
|
reasons: list[str] = []
|
||||||
if infra_stop:
|
if infra_stop:
|
||||||
reasons.append("infra_stop is active; PR selection/review/merge is forbidden")
|
reasons.append("infra_stop is active; PR selection/review/merge is forbidden")
|
||||||
@@ -104,6 +112,12 @@ def assess_workflow_blockers(
|
|||||||
reasons.append("MCP reconnect failed; stale session state cannot be reused")
|
reasons.append("MCP reconnect failed; stale session state cannot be reused")
|
||||||
if stale_capability_state:
|
if stale_capability_state:
|
||||||
reasons.append("stale MCP capability state detected after reconnect failure")
|
reasons.append("stale MCP capability state detected after reconnect failure")
|
||||||
|
if live_namespace_broken:
|
||||||
|
reasons.append(
|
||||||
|
"live MCP namespace call path is broken (registered in FastMCP but "
|
||||||
|
"not callable through the namespace); repair the namespace before "
|
||||||
|
"review/merge"
|
||||||
|
)
|
||||||
return {
|
return {
|
||||||
"block": bool(reasons),
|
"block": bool(reasons),
|
||||||
"reasons": reasons,
|
"reasons": reasons,
|
||||||
@@ -118,16 +132,19 @@ def assess_state_advancement(
|
|||||||
state_completion: dict[str, bool] | None,
|
state_completion: dict[str, bool] | None,
|
||||||
*,
|
*,
|
||||||
target_state: str,
|
target_state: str,
|
||||||
infra_stop: bool = False,
|
**blocker_kwargs,
|
||||||
capability_blocked: bool = False,
|
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Fail closed when *target_state* is requested before upstream gates pass."""
|
"""Fail closed when *target_state* is requested before upstream gates pass.
|
||||||
|
|
||||||
|
Forwards every blocker flag (``infra_stop``, ``capability_blocked``,
|
||||||
|
``mcp_reconnect_failed``, ``stale_capability_state``,
|
||||||
|
``live_namespace_broken``) to :func:`assess_workflow_blockers` so
|
||||||
|
``can_approve``/``can_merge`` honor the full blocker set, not just
|
||||||
|
infra_stop/capability_blocked (#543 AC5).
|
||||||
|
"""
|
||||||
completion = dict(state_completion or {})
|
completion = dict(state_completion or {})
|
||||||
target = _clean(target_state).upper()
|
target = _clean(target_state).upper()
|
||||||
blockers = assess_workflow_blockers(
|
blockers = assess_workflow_blockers(**blocker_kwargs)
|
||||||
infra_stop=infra_stop,
|
|
||||||
capability_blocked=capability_blocked,
|
|
||||||
)
|
|
||||||
reasons = list(blockers["reasons"])
|
reasons = list(blockers["reasons"])
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import unittest
|
|||||||
|
|
||||||
import gitea_mcp_server
|
import gitea_mcp_server
|
||||||
import mcp_namespace_health
|
import mcp_namespace_health
|
||||||
|
import review_merge_state_machine as rmsm
|
||||||
|
|
||||||
|
|
||||||
class TestMcpNamespaceHealth(unittest.TestCase):
|
class TestMcpNamespaceHealth(unittest.TestCase):
|
||||||
@@ -81,5 +82,74 @@ class TestMcpNamespaceHealth(unittest.TestCase):
|
|||||||
self.assertEqual(result["diagnostics"]["process_pid"], 9876)
|
self.assertEqual(result["diagnostics"]["process_pid"], 9876)
|
||||||
|
|
||||||
|
|
||||||
|
class TestLiveNamespaceBlocksMerge(unittest.TestCase):
|
||||||
|
"""AC5: a broken live namespace must hard-block the review/merge state machine."""
|
||||||
|
|
||||||
|
def _merge_ready_completion(self):
|
||||||
|
# Completion through PRE_MERGE_RECHECK is the state where a clean merge
|
||||||
|
# is otherwise allowed (see test_merge_allowed_with_pre_merge_gates).
|
||||||
|
idx = rmsm.REVIEW_MERGE_STATES.index("PRE_MERGE_RECHECK")
|
||||||
|
return {state: True for state in rmsm.REVIEW_MERGE_STATES[: idx + 1]}
|
||||||
|
|
||||||
|
def _all_gates(self):
|
||||||
|
return {gate: True for gate in rmsm._PRE_MERGE_REQUIRED_GATES}
|
||||||
|
|
||||||
|
def test_assess_workflow_blockers_flags_live_namespace_broken(self):
|
||||||
|
clean = rmsm.assess_workflow_blockers()
|
||||||
|
self.assertFalse(clean["block"])
|
||||||
|
|
||||||
|
broken = rmsm.assess_workflow_blockers(live_namespace_broken=True)
|
||||||
|
self.assertTrue(broken["block"])
|
||||||
|
self.assertTrue(any("namespace" in r.lower() for r in broken["reasons"]))
|
||||||
|
|
||||||
|
def test_can_merge_blocks_even_when_all_gates_pass(self):
|
||||||
|
# Without the namespace blocker a fully-complete workflow can merge...
|
||||||
|
allowed = rmsm.can_merge(
|
||||||
|
self._merge_ready_completion(), pre_merge_gates=self._all_gates()
|
||||||
|
)
|
||||||
|
self.assertTrue(allowed["allowed"])
|
||||||
|
|
||||||
|
# ...but a broken live namespace overrides every satisfied gate.
|
||||||
|
blocked = rmsm.can_merge(
|
||||||
|
self._merge_ready_completion(),
|
||||||
|
pre_merge_gates=self._all_gates(),
|
||||||
|
live_namespace_broken=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(blocked["block"])
|
||||||
|
self.assertFalse(blocked["allowed"])
|
||||||
|
|
||||||
|
def test_workflow_status_forwards_namespace_blocker(self):
|
||||||
|
status = rmsm.workflow_status(
|
||||||
|
self._merge_ready_completion(), live_namespace_broken=True
|
||||||
|
)
|
||||||
|
self.assertFalse(status["merge_allowed"])
|
||||||
|
self.assertFalse(status["approve_allowed"])
|
||||||
|
|
||||||
|
def test_server_tool_blocks_merge_on_live_namespace_broken(self):
|
||||||
|
result = gitea_mcp_server.gitea_assess_review_merge_state_machine(
|
||||||
|
state_completion=self._merge_ready_completion(),
|
||||||
|
pre_merge_gates=self._all_gates(),
|
||||||
|
live_namespace_broken=True,
|
||||||
|
)
|
||||||
|
self.assertTrue(result["blockers"]["block"])
|
||||||
|
self.assertFalse(result["merge"]["allowed"])
|
||||||
|
|
||||||
|
def test_classify_verdict_bridges_into_merge_block(self):
|
||||||
|
# The classify verdict is the intended feed for live_namespace_broken.
|
||||||
|
verdict = mcp_namespace_health.classify_namespace_probe(
|
||||||
|
"gitea-merger",
|
||||||
|
registered_tools=["gitea_whoami", "gitea_adopt_merger_pr_lease"],
|
||||||
|
probe_result={"success": False, "error": "client is closing: EOF"},
|
||||||
|
process={"pid": 555, "profile": "prgs-merger"},
|
||||||
|
)
|
||||||
|
self.assertTrue(verdict["blocks_merge_workflow"])
|
||||||
|
blocked = rmsm.can_merge(
|
||||||
|
self._merge_ready_completion(),
|
||||||
|
pre_merge_gates=self._all_gates(),
|
||||||
|
live_namespace_broken=verdict["blocks_merge_workflow"],
|
||||||
|
)
|
||||||
|
self.assertFalse(blocked["allowed"])
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user