Merge branch 'master' into feat/issue-637-timeline-model
# Conflicts: # webui/app.py
This commit is contained in:
@@ -0,0 +1,581 @@
|
||||
"""Authoritative controller cross-role generic queue allocation (#840)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from allocator_service import (
|
||||
ALLOCATION_MODE_CROSS_ROLE,
|
||||
ALLOCATION_MODE_ROLE_SCOPED,
|
||||
OUTCOME_NO_SAFE,
|
||||
OUTCOME_PREVIEW,
|
||||
OUTCOME_WAIT,
|
||||
ROLE_AUTHOR,
|
||||
ROLE_CONTROLLER,
|
||||
ROLE_MERGER,
|
||||
ROLE_RECONCILER,
|
||||
ROLE_REVIEWER,
|
||||
WorkCandidate,
|
||||
allocate_next_work,
|
||||
build_selection_dict,
|
||||
classify_skip,
|
||||
required_namespace_for_role,
|
||||
required_profile_for_role,
|
||||
resolve_allocation_mode,
|
||||
selected_action_for_candidate,
|
||||
)
|
||||
from control_plane_db import ControlPlaneDB
|
||||
import role_session_router
|
||||
from role_session_router import (
|
||||
ROUTE_ALLOWED,
|
||||
ROUTE_AMBIGUOUS,
|
||||
ROUTE_WRONG_ROLE,
|
||||
route_task_session,
|
||||
)
|
||||
import namespace_workspace_binding as nwb
|
||||
import task_capability_map
|
||||
|
||||
|
||||
class CrossRoleAllocationModeTest(unittest.TestCase):
|
||||
def test_controller_defaults_to_cross_role(self) -> None:
|
||||
self.assertEqual(
|
||||
resolve_allocation_mode(ROLE_CONTROLLER),
|
||||
ALLOCATION_MODE_CROSS_ROLE,
|
||||
)
|
||||
|
||||
def test_worker_defaults_to_role_scoped(self) -> None:
|
||||
for role in (ROLE_AUTHOR, ROLE_REVIEWER, ROLE_MERGER, ROLE_RECONCILER):
|
||||
self.assertEqual(
|
||||
resolve_allocation_mode(role),
|
||||
ALLOCATION_MODE_ROLE_SCOPED,
|
||||
)
|
||||
|
||||
def test_explicit_modes(self) -> None:
|
||||
self.assertEqual(
|
||||
resolve_allocation_mode(ROLE_CONTROLLER, "role_scoped"),
|
||||
ALLOCATION_MODE_ROLE_SCOPED,
|
||||
)
|
||||
self.assertEqual(
|
||||
resolve_allocation_mode(ROLE_AUTHOR, "cross_role"),
|
||||
ALLOCATION_MODE_CROSS_ROLE,
|
||||
)
|
||||
|
||||
|
||||
class CrossRoleSelectionPayloadTest(unittest.TestCase):
|
||||
def test_selection_contains_required_fields(self) -> None:
|
||||
c = WorkCandidate(
|
||||
kind="issue",
|
||||
number=840,
|
||||
labels=("status:ready",),
|
||||
title="cross-role",
|
||||
priority=20,
|
||||
)
|
||||
sel = build_selection_dict(
|
||||
c,
|
||||
active_role=ROLE_CONTROLLER,
|
||||
required_role=ROLE_AUTHOR,
|
||||
profile_name="prgs-controller",
|
||||
allocation_mode=ALLOCATION_MODE_CROSS_ROLE,
|
||||
)
|
||||
self.assertEqual(sel["number"], 840)
|
||||
self.assertEqual(sel["kind"], "issue")
|
||||
self.assertEqual(sel["required_role"], ROLE_AUTHOR)
|
||||
self.assertEqual(sel["selected_action"], "implement")
|
||||
self.assertEqual(sel["action"], "implement")
|
||||
self.assertEqual(sel["required_profile"], "prgs-author")
|
||||
self.assertEqual(sel["required_namespace"], "gitea-author")
|
||||
self.assertEqual(sel["pinned"]["number"], 840)
|
||||
self.assertIsNone(sel["pinned"]["head_sha"])
|
||||
|
||||
def test_profile_prefix_preserved(self) -> None:
|
||||
self.assertEqual(
|
||||
required_profile_for_role(ROLE_REVIEWER, profile_name="dadeschools-controller"),
|
||||
"dadeschools-reviewer",
|
||||
)
|
||||
self.assertEqual(
|
||||
required_namespace_for_role(ROLE_MERGER),
|
||||
"gitea-merger",
|
||||
)
|
||||
|
||||
def test_selected_actions_per_role(self) -> None:
|
||||
issue = WorkCandidate(kind="issue", number=1, labels=("status:ready",))
|
||||
pr_review = WorkCandidate(kind="pr", number=2, head_sha="a" * 40)
|
||||
pr_rc = WorkCandidate(
|
||||
kind="pr",
|
||||
number=3,
|
||||
head_sha="b" * 40,
|
||||
request_changes_current_head=True,
|
||||
)
|
||||
pr_merge = WorkCandidate(
|
||||
kind="pr",
|
||||
number=4,
|
||||
head_sha="c" * 40,
|
||||
approval_on_current_head=True,
|
||||
mergeable=True,
|
||||
)
|
||||
pr_recon = WorkCandidate(
|
||||
kind="pr",
|
||||
number=5,
|
||||
head_sha="d" * 40,
|
||||
approval_contaminated=True,
|
||||
)
|
||||
self.assertEqual(selected_action_for_candidate(issue, ROLE_AUTHOR), "implement")
|
||||
self.assertEqual(
|
||||
selected_action_for_candidate(pr_rc, ROLE_AUTHOR),
|
||||
"address_pr_change_requests",
|
||||
)
|
||||
self.assertEqual(
|
||||
selected_action_for_candidate(pr_review, ROLE_REVIEWER), "review"
|
||||
)
|
||||
self.assertEqual(selected_action_for_candidate(pr_merge, ROLE_MERGER), "merge")
|
||||
self.assertEqual(
|
||||
selected_action_for_candidate(pr_recon, ROLE_RECONCILER),
|
||||
"reconcile_contaminated_approval",
|
||||
)
|
||||
|
||||
|
||||
class CrossRoleAllocateServiceTest(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.db = ControlPlaneDB(os.path.join(self._tmp.name, "cp.sqlite3"))
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self._tmp.cleanup()
|
||||
|
||||
def _alloc(self, **kwargs):
|
||||
defaults = dict(
|
||||
db=self.db,
|
||||
session_id="ctrl-session",
|
||||
role=ROLE_CONTROLLER,
|
||||
remote="prgs",
|
||||
org="org",
|
||||
repo="repo",
|
||||
candidates=[],
|
||||
apply=False,
|
||||
profile_name="prgs-controller",
|
||||
username="controller-bot",
|
||||
controller_instance_id="ctrl-1",
|
||||
)
|
||||
defaults.update(kwargs)
|
||||
return allocate_next_work(**defaults)
|
||||
|
||||
def test_eligible_author_work(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=100,
|
||||
labels=("status:ready",),
|
||||
title="author work",
|
||||
priority=20,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["outcome"], OUTCOME_PREVIEW)
|
||||
self.assertEqual(res["allocation_mode"], ALLOCATION_MODE_CROSS_ROLE)
|
||||
self.assertIsNotNone(res["selected"])
|
||||
self.assertEqual(res["selected"]["number"], 100)
|
||||
self.assertEqual(res["required_role"], ROLE_AUTHOR)
|
||||
self.assertEqual(res["selected_action"], "implement")
|
||||
self.assertEqual(res["required_profile"], "prgs-author")
|
||||
self.assertEqual(res["required_namespace"], "gitea-author")
|
||||
self.assertIn("allocate", res["controller_allowed_actions"])
|
||||
self.assertIn("merge", res["controller_forbidden_actions"])
|
||||
self.assertFalse(res["allocation_evidence"]["lease_created"])
|
||||
|
||||
def test_eligible_reviewer_work(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=200,
|
||||
head_sha="e" * 40,
|
||||
title="needs review",
|
||||
priority=30,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["selected"]["number"], 200)
|
||||
self.assertEqual(res["required_role"], ROLE_REVIEWER)
|
||||
self.assertEqual(res["selected_action"], "review")
|
||||
self.assertEqual(res["required_profile"], "prgs-reviewer")
|
||||
self.assertEqual(res["selected"]["pinned"]["head_sha"], "e" * 40)
|
||||
|
||||
def test_eligible_merger_work(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=300,
|
||||
head_sha="f" * 40,
|
||||
approval_on_current_head=True,
|
||||
mergeable=True,
|
||||
priority=40,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["selected"]["number"], 300)
|
||||
self.assertEqual(res["required_role"], ROLE_MERGER)
|
||||
self.assertEqual(res["selected_action"], "merge")
|
||||
|
||||
def test_eligible_reconciler_work(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=400,
|
||||
head_sha="1" * 40,
|
||||
approval_contaminated=True,
|
||||
priority=50,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["selected"]["number"], 400)
|
||||
self.assertEqual(res["required_role"], ROLE_RECONCILER)
|
||||
self.assertIn("reconcile", res["selected_action"])
|
||||
|
||||
def test_no_eligible_work(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=10,
|
||||
labels=("status:blocked",),
|
||||
blocked=True,
|
||||
priority=99,
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=11,
|
||||
labels=("status:ready",),
|
||||
dependency_unmet=True,
|
||||
dependency_reason="blocked by #10",
|
||||
priority=98,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["outcome"], OUTCOME_NO_SAFE)
|
||||
self.assertIsNone(res["selected"])
|
||||
self.assertEqual(res["allocation_mode"], ALLOCATION_MODE_CROSS_ROLE)
|
||||
|
||||
def test_leased_work_skipped(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=50,
|
||||
labels=("status:ready",),
|
||||
priority=20,
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=51,
|
||||
labels=("status:ready",),
|
||||
priority=10,
|
||||
),
|
||||
]
|
||||
# Seed a foreign lease on issue 50 via assign_and_lease under another session.
|
||||
other = allocate_next_work(
|
||||
self.db,
|
||||
session_id="other-worker",
|
||||
role=ROLE_AUTHOR,
|
||||
remote="prgs",
|
||||
org="org",
|
||||
repo="repo",
|
||||
candidates=cands[:1],
|
||||
apply=True,
|
||||
profile_name="prgs-author",
|
||||
controller_instance_id="other-ctrl",
|
||||
)
|
||||
self.assertEqual(other["outcome"], "assigned_work")
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertIsNotNone(res["selected"])
|
||||
self.assertEqual(res["selected"]["number"], 51)
|
||||
self.assertTrue(any(s["number"] == 50 for s in res["skipped"]))
|
||||
self.assertTrue(res["claims_excluded"])
|
||||
|
||||
def test_dependencies_skipped(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=1,
|
||||
labels=("status:ready",),
|
||||
priority=99,
|
||||
dependency_unmet=True,
|
||||
dependency_reason="needs #2",
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=2,
|
||||
labels=("status:ready",),
|
||||
priority=1,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["selected"]["number"], 2)
|
||||
skipped = {s["number"]: s["reason"] for s in res["skipped"]}
|
||||
self.assertIn(1, skipped)
|
||||
self.assertIn("needs #2", skipped[1])
|
||||
|
||||
def test_pagination_limit_only_truncates_skip_report(self) -> None:
|
||||
"""Ranking uses full inventory; reporting limit is MCP-layer only.
|
||||
|
||||
Service ranks all candidates; prove higher-priority eligible item
|
||||
wins even when many skipped precede it.
|
||||
"""
|
||||
cands = []
|
||||
for n in range(1, 30):
|
||||
cands.append(
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=n,
|
||||
labels=("status:ready",),
|
||||
priority=100 - n,
|
||||
dependency_unmet=True,
|
||||
dependency_reason=f"dep {n}",
|
||||
)
|
||||
)
|
||||
cands.append(
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=999,
|
||||
labels=("status:ready",),
|
||||
priority=1,
|
||||
)
|
||||
)
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["selected"]["number"], 999)
|
||||
self.assertGreaterEqual(len(res["skipped"]), 29)
|
||||
|
||||
def test_role_scoped_controller_legacy_still_restricts(self) -> None:
|
||||
"""role_scoped controller only takes reconciler-needed items."""
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=1,
|
||||
labels=("status:ready",),
|
||||
priority=50,
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=2,
|
||||
head_sha="a" * 40,
|
||||
approval_contaminated=True,
|
||||
priority=1,
|
||||
),
|
||||
]
|
||||
res = self._alloc(
|
||||
candidates=cands,
|
||||
allocation_mode=ALLOCATION_MODE_ROLE_SCOPED,
|
||||
)
|
||||
self.assertEqual(res["allocation_mode"], ALLOCATION_MODE_ROLE_SCOPED)
|
||||
self.assertEqual(res["selected"]["number"], 2)
|
||||
self.assertEqual(res["required_role"], ROLE_RECONCILER)
|
||||
|
||||
def test_cross_role_prefers_highest_priority_across_roles(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=10,
|
||||
labels=("status:ready",),
|
||||
priority=10,
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=20,
|
||||
head_sha="b" * 40,
|
||||
priority=50,
|
||||
),
|
||||
WorkCandidate(
|
||||
kind="pr",
|
||||
number=30,
|
||||
head_sha="c" * 40,
|
||||
approval_on_current_head=True,
|
||||
mergeable=True,
|
||||
priority=20,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
# PR #20 highest priority → reviewer
|
||||
self.assertEqual(res["selected"]["number"], 20)
|
||||
self.assertEqual(res["required_role"], ROLE_REVIEWER)
|
||||
|
||||
def test_apply_creates_lease_evidence_for_required_role(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=777,
|
||||
labels=("status:ready",),
|
||||
priority=20,
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands, apply=True)
|
||||
self.assertEqual(res["outcome"], "assigned_work")
|
||||
self.assertTrue(res["allocation_evidence"]["lease_created"])
|
||||
self.assertEqual(res["allocation_evidence"]["lease_role"], ROLE_AUTHOR)
|
||||
proof = res["lease_proof"]
|
||||
self.assertIsNotNone(proof["lease_id"])
|
||||
self.assertEqual(proof["lease_role"], ROLE_AUTHOR)
|
||||
self.assertIn("implement", proof["allowed_actions"])
|
||||
# Controller isolation: controller still forbids merge/push/create_pr
|
||||
self.assertIn("merge", res["controller_forbidden_actions"])
|
||||
self.assertIn("push", res["controller_forbidden_actions"])
|
||||
|
||||
def test_metadata_consistency_role_is_controller(self) -> None:
|
||||
cands = [
|
||||
WorkCandidate(
|
||||
kind="issue",
|
||||
number=1,
|
||||
labels=("status:ready",),
|
||||
),
|
||||
]
|
||||
res = self._alloc(candidates=cands)
|
||||
self.assertEqual(res["role"], ROLE_CONTROLLER)
|
||||
self.assertEqual(res["routing_role"], ROLE_CONTROLLER)
|
||||
self.assertEqual(res["required_role"], ROLE_AUTHOR)
|
||||
|
||||
|
||||
class ProcessWorkQueueRouterTest(unittest.TestCase):
|
||||
def tearDown(self) -> None:
|
||||
role_session_router.clear_route_state()
|
||||
|
||||
def test_process_work_queue_allowed_for_controller(self) -> None:
|
||||
res = route_task_session(
|
||||
"process_work_queue",
|
||||
active_profile="prgs-controller",
|
||||
active_role_kind="controller",
|
||||
allowed_in_current_session=True,
|
||||
)
|
||||
self.assertEqual(res["route_result"], ROUTE_ALLOWED)
|
||||
self.assertEqual(res["required_role"], "controller")
|
||||
self.assertTrue(res["downstream_allowed"])
|
||||
|
||||
def test_process_work_queue_hyphen_alias(self) -> None:
|
||||
res = route_task_session(
|
||||
"process-work-queue",
|
||||
active_profile="prgs-controller",
|
||||
active_role_kind="controller",
|
||||
allowed_in_current_session=True,
|
||||
)
|
||||
self.assertEqual(res["route_result"], ROUTE_ALLOWED)
|
||||
|
||||
def test_process_work_queue_wrong_role_for_author(self) -> None:
|
||||
res = route_task_session(
|
||||
"process_work_queue",
|
||||
active_profile="prgs-author",
|
||||
active_role_kind="author",
|
||||
allowed_in_current_session=False,
|
||||
)
|
||||
self.assertEqual(res["route_result"], ROUTE_WRONG_ROLE)
|
||||
self.assertEqual(res["required_role"], "controller")
|
||||
self.assertFalse(res["downstream_allowed"])
|
||||
|
||||
def test_unknown_still_ambiguous(self) -> None:
|
||||
res = route_task_session(
|
||||
"not_a_real_task",
|
||||
active_profile="prgs-controller",
|
||||
active_role_kind="controller",
|
||||
allowed_in_current_session=False,
|
||||
)
|
||||
self.assertEqual(res["route_result"], ROUTE_AMBIGUOUS)
|
||||
|
||||
def test_capability_map_process_work_queue_is_controller(self) -> None:
|
||||
self.assertEqual(
|
||||
task_capability_map.required_role("process_work_queue"),
|
||||
"controller",
|
||||
)
|
||||
self.assertEqual(
|
||||
task_capability_map.required_permission("process_work_queue"),
|
||||
"gitea.read",
|
||||
)
|
||||
|
||||
|
||||
class ControllerRoleMetadataTest(unittest.TestCase):
|
||||
def test_normalize_role_kind_controller(self) -> None:
|
||||
self.assertEqual(
|
||||
nwb.normalize_role_kind("controller"),
|
||||
"controller",
|
||||
)
|
||||
self.assertEqual(
|
||||
nwb.normalize_role_kind("author", profile_name="prgs-controller"),
|
||||
"controller",
|
||||
)
|
||||
self.assertEqual(
|
||||
nwb.normalize_role_kind("reconciler", profile_name="prgs-controller"),
|
||||
"controller",
|
||||
)
|
||||
|
||||
def test_profile_role_kind_prefers_declared_controller(self) -> None:
|
||||
# Import from worktree package path via sys.path already set by pytest.
|
||||
import gitea_mcp_server as mcp
|
||||
|
||||
profile = {
|
||||
"profile_name": "prgs-controller",
|
||||
"role": "controller",
|
||||
"allowed_operations": [
|
||||
"gitea.read",
|
||||
"gitea.issue.comment",
|
||||
"gitea.pr.close",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve",
|
||||
"gitea.pr.merge",
|
||||
"gitea.pr.create",
|
||||
"gitea.branch.push",
|
||||
],
|
||||
}
|
||||
# Declared role wins even if permissions look reconciler-like.
|
||||
self.assertEqual(mcp._profile_role_kind(profile), "controller")
|
||||
# Name-based fallback.
|
||||
profile_no_role = dict(profile)
|
||||
profile_no_role["role"] = None
|
||||
profile_no_role["role_kind"] = None
|
||||
self.assertEqual(mcp._profile_role_kind(profile_no_role), "controller")
|
||||
|
||||
def test_permission_inference_without_controller_name_stays_reconciler(self) -> None:
|
||||
import gitea_mcp_server as mcp
|
||||
|
||||
# Pure permission inference still may return reconciler when no controller
|
||||
# declaration exists — that is intentional for reconciler profiles.
|
||||
role = mcp._role_kind(
|
||||
["gitea.read", "gitea.pr.close", "gitea.issue.comment"],
|
||||
["gitea.pr.approve", "gitea.pr.merge", "gitea.pr.create", "gitea.branch.push"],
|
||||
)
|
||||
self.assertEqual(role, "reconciler")
|
||||
|
||||
|
||||
class DashboardRemainsExplanatoryTest(unittest.TestCase):
|
||||
def test_dashboard_prompt_points_at_allocator_not_self_select(self) -> None:
|
||||
import workflow_dashboard as wd
|
||||
|
||||
self.assertIn("gitea_allocate_next_work", wd.PROMPT_CONTROLLER)
|
||||
self.assertIn("process_work_queue", wd.PROMPT_CONTROLLER)
|
||||
self.assertIn("never replaces allocator", wd.PROMPT_CONTROLLER.lower())
|
||||
self.assertNotIn("self-select", wd.PROMPT_CONTROLLER.lower())
|
||||
|
||||
|
||||
class ClassifySkipCrossRoleTest(unittest.TestCase):
|
||||
def test_controller_cross_role_accepts_author_issue(self) -> None:
|
||||
c = WorkCandidate(kind="issue", number=1, labels=("status:ready",))
|
||||
self.assertIsNone(
|
||||
classify_skip(
|
||||
c,
|
||||
role=ROLE_CONTROLLER,
|
||||
terminal_pr=None,
|
||||
allocation_mode=ALLOCATION_MODE_CROSS_ROLE,
|
||||
)
|
||||
)
|
||||
|
||||
def test_legacy_controller_skips_author_issue(self) -> None:
|
||||
c = WorkCandidate(kind="issue", number=1, labels=("status:ready",))
|
||||
reason = classify_skip(
|
||||
c,
|
||||
role=ROLE_CONTROLLER,
|
||||
terminal_pr=None,
|
||||
allocation_mode=ALLOCATION_MODE_ROLE_SCOPED,
|
||||
)
|
||||
self.assertIsNotNone(reason)
|
||||
self.assertIn("does not require controller", reason or "")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,499 @@
|
||||
"""Tests for the read-only system-health API (#634).
|
||||
|
||||
Covers the acceptance criteria directly: a structured payload with readiness
|
||||
and a dependency list (AC1), version and uptime when knowable (AC2), stale
|
||||
runtime reported without a false mutation-safe claim (AC3), and the healthy /
|
||||
degraded-dependency / redaction cases (AC4).
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
import sqlite3
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest import mock
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from starlette.testclient import TestClient
|
||||
|
||||
import control_plane_db
|
||||
from webui.app import create_app
|
||||
from webui.deployment_boundary import scan_text_for_client_secrets
|
||||
from webui.system_health import (
|
||||
API_PATH,
|
||||
STATUS_DEGRADED,
|
||||
STATUS_DOWN,
|
||||
STATUS_OK,
|
||||
STATUS_SKIPPED,
|
||||
DependencyProbe,
|
||||
StaleRuntime,
|
||||
assess_stale_runtime,
|
||||
clear_probe_cache,
|
||||
load_system_health,
|
||||
namespace_summaries,
|
||||
probe_control_plane_db,
|
||||
probe_gitea,
|
||||
process_uptime,
|
||||
redact,
|
||||
redact_url,
|
||||
snapshot_to_dict,
|
||||
)
|
||||
|
||||
|
||||
def _probe(name, status, *, required=True, detail="detail", kind="test"):
|
||||
return DependencyProbe(
|
||||
name=name,
|
||||
kind=kind,
|
||||
status=status,
|
||||
detail=detail,
|
||||
required=required,
|
||||
latency_ms=1.5,
|
||||
metadata={},
|
||||
)
|
||||
|
||||
|
||||
_ALL_HEALTHY = (
|
||||
_probe("control_plane_db", STATUS_OK, kind="sqlite"),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
_probe("gitea", STATUS_OK, required=False, kind="http"),
|
||||
)
|
||||
|
||||
_CLEAN_PARITY = StaleRuntime(
|
||||
daemon_head="abc123",
|
||||
checkout_head="abc123",
|
||||
remote_head="abc123",
|
||||
stale=False,
|
||||
determinable=True,
|
||||
mutation_safe=True,
|
||||
reasons=(),
|
||||
)
|
||||
|
||||
|
||||
class CleanParityMixin:
|
||||
"""Pin parity for tests about aggregation rather than staleness.
|
||||
|
||||
Without this the assertions depend on the real checkout: a worktree whose
|
||||
branch is ahead of its upstream is genuinely stale, which would degrade the
|
||||
overall status and make these cases fail for an unrelated reason.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
patcher = mock.patch(
|
||||
"webui.system_health.assess_stale_runtime",
|
||||
return_value=_CLEAN_PARITY,
|
||||
)
|
||||
patcher.start()
|
||||
self.addCleanup(patcher.stop)
|
||||
|
||||
|
||||
class TestDependencyAggregation(CleanParityMixin, unittest.TestCase):
|
||||
"""AC1 — readiness and dependency list derived from probe results."""
|
||||
|
||||
def test_all_healthy_is_ok_and_ready(self):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc123")
|
||||
self.assertEqual(snapshot.status, STATUS_OK)
|
||||
self.assertTrue(snapshot.ready)
|
||||
self.assertTrue(snapshot.readiness_complete)
|
||||
self.assertEqual(snapshot.readiness_reasons, ())
|
||||
self.assertEqual(len(snapshot.dependencies), 3)
|
||||
|
||||
def test_required_dependency_down_blocks_readiness(self):
|
||||
probes = (
|
||||
_probe("control_plane_db", STATUS_DOWN, detail="file missing", kind="sqlite"),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
_probe("gitea", STATUS_OK, required=False, kind="http"),
|
||||
)
|
||||
snapshot = load_system_health(probes=probes, daemon_head="abc123")
|
||||
self.assertEqual(snapshot.status, STATUS_DOWN)
|
||||
self.assertFalse(snapshot.ready)
|
||||
self.assertTrue(
|
||||
any("control_plane_db" in reason for reason in snapshot.readiness_reasons)
|
||||
)
|
||||
|
||||
def test_optional_dependency_down_degrades_but_stays_ready(self):
|
||||
"""A failing optional probe must not claim the process itself is unready."""
|
||||
probes = (
|
||||
_probe("control_plane_db", STATUS_OK, kind="sqlite"),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
_probe("gitea", STATUS_DOWN, required=False, detail="timeout", kind="http"),
|
||||
)
|
||||
snapshot = load_system_health(probes=probes, daemon_head="abc123")
|
||||
self.assertEqual(snapshot.status, STATUS_DEGRADED)
|
||||
self.assertTrue(snapshot.ready)
|
||||
self.assertTrue(any("gitea" in reason for reason in snapshot.readiness_reasons))
|
||||
|
||||
def test_unrun_required_probe_leaves_readiness_incomplete(self):
|
||||
"""Not probed is not the same as passing."""
|
||||
probes = (
|
||||
_probe("control_plane_db", STATUS_OK, kind="sqlite"),
|
||||
_probe("repository", STATUS_SKIPPED, detail="offline", kind="git"),
|
||||
)
|
||||
snapshot = load_system_health(probes=probes, daemon_head="abc123")
|
||||
self.assertFalse(snapshot.ready)
|
||||
self.assertFalse(snapshot.readiness_complete)
|
||||
self.assertEqual(snapshot.status, STATUS_DEGRADED)
|
||||
|
||||
def test_skipped_optional_probe_does_not_block_readiness(self):
|
||||
probes = (
|
||||
_probe("control_plane_db", STATUS_OK, kind="sqlite"),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
_probe("gitea", STATUS_SKIPPED, required=False, kind="http"),
|
||||
)
|
||||
snapshot = load_system_health(probes=probes, daemon_head="abc123")
|
||||
self.assertTrue(snapshot.ready)
|
||||
self.assertTrue(snapshot.readiness_complete)
|
||||
|
||||
|
||||
class TestVersionAndUptime(CleanParityMixin, unittest.TestCase):
|
||||
"""AC2 — version and uptime present when knowable."""
|
||||
|
||||
def test_uptime_and_start_time_present(self):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc123")
|
||||
self.assertGreaterEqual(snapshot.uptime_seconds, 0.0)
|
||||
self.assertIn("T", snapshot.started_at)
|
||||
|
||||
def test_process_uptime_helper_matches_shape(self):
|
||||
started_at, uptime = process_uptime()
|
||||
self.assertIn("T", started_at)
|
||||
self.assertGreaterEqual(uptime, 0.0)
|
||||
|
||||
def test_version_reports_python_and_schema_version(self):
|
||||
probes = (
|
||||
DependencyProbe(
|
||||
name="control_plane_db",
|
||||
kind="sqlite",
|
||||
status=STATUS_OK,
|
||||
detail="ok",
|
||||
required=True,
|
||||
latency_ms=1.0,
|
||||
metadata={"schema_version": control_plane_db.SCHEMA_VERSION},
|
||||
),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
)
|
||||
snapshot = load_system_health(probes=probes, daemon_head="abc123")
|
||||
self.assertEqual(
|
||||
snapshot.version.control_plane_schema_version,
|
||||
control_plane_db.SCHEMA_VERSION,
|
||||
)
|
||||
self.assertTrue(snapshot.version.python_version)
|
||||
|
||||
def test_version_known_flag_false_when_sha_unavailable(self):
|
||||
with mock.patch("webui.system_health._git", return_value=None):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc")
|
||||
self.assertIsNone(snapshot.version.git_sha)
|
||||
self.assertFalse(snapshot.version.known)
|
||||
|
||||
|
||||
class TestStaleRuntime(unittest.TestCase):
|
||||
"""AC3 — stale runtime reflected without a false mutation-safe claim."""
|
||||
|
||||
def test_matching_commits_are_mutation_safe(self):
|
||||
assessment = assess_stale_runtime(
|
||||
Path("/tmp"),
|
||||
daemon_head="aaa",
|
||||
git_reader=lambda *args: "aaa",
|
||||
)
|
||||
self.assertFalse(assessment.stale)
|
||||
self.assertTrue(assessment.determinable)
|
||||
self.assertTrue(assessment.mutation_safe)
|
||||
|
||||
def test_diverged_commits_are_stale_and_not_mutation_safe(self):
|
||||
reads = {"HEAD": "aaa", "@{upstream}": "bbb"}
|
||||
assessment = assess_stale_runtime(
|
||||
Path("/tmp"),
|
||||
daemon_head="aaa",
|
||||
git_reader=lambda *args: reads.get(args[-1]),
|
||||
)
|
||||
self.assertTrue(assessment.stale)
|
||||
self.assertFalse(assessment.mutation_safe)
|
||||
self.assertTrue(assessment.reasons)
|
||||
|
||||
def test_unknown_remote_is_not_mutation_safe(self):
|
||||
"""Indeterminate must never read as safe."""
|
||||
reads = {"HEAD": "aaa", "@{upstream}": None}
|
||||
assessment = assess_stale_runtime(
|
||||
Path("/tmp"),
|
||||
daemon_head="aaa",
|
||||
git_reader=lambda *args: reads.get(args[-1]),
|
||||
)
|
||||
self.assertFalse(assessment.determinable)
|
||||
self.assertFalse(assessment.mutation_safe)
|
||||
self.assertFalse(assessment.stale)
|
||||
self.assertTrue(
|
||||
any("indeterminate" in reason for reason in assessment.reasons)
|
||||
)
|
||||
|
||||
def test_unobservable_daemon_head_is_disclosed(self):
|
||||
assessment = assess_stale_runtime(
|
||||
Path("/tmp"),
|
||||
git_reader=lambda *args: "aaa",
|
||||
)
|
||||
self.assertTrue(
|
||||
any("not observable" in reason for reason in assessment.reasons)
|
||||
)
|
||||
|
||||
def test_stale_runtime_degrades_overall_status(self):
|
||||
reads = {"HEAD": "aaa", "@{upstream}": "bbb"}
|
||||
# Pinned rather than inherited: this path uses the default git reader,
|
||||
# so the assertion must hold whether or not the suite runs offline.
|
||||
with mock.patch.dict(os.environ, {"WEBUI_TEST_OFFLINE": ""}), mock.patch(
|
||||
"webui.system_health._git",
|
||||
side_effect=lambda repo, *args: reads.get(args[-1]),
|
||||
):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="aaa")
|
||||
self.assertTrue(snapshot.stale_runtime.stale)
|
||||
self.assertFalse(snapshot.stale_runtime.mutation_safe)
|
||||
self.assertEqual(snapshot.status, STATUS_DEGRADED)
|
||||
|
||||
|
||||
class TestControlPlaneDbProbe(unittest.TestCase):
|
||||
"""The required local dependency, probed read-only."""
|
||||
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.tmp.cleanup)
|
||||
self.db_path = str(Path(self.tmp.name) / "control-plane.db")
|
||||
|
||||
def _build_db(self, schema_version):
|
||||
conn = sqlite3.connect(self.db_path)
|
||||
conn.execute("CREATE TABLE schema_meta (key TEXT PRIMARY KEY, value TEXT)")
|
||||
conn.execute("CREATE TABLE leases (lease_id TEXT PRIMARY KEY, status TEXT)")
|
||||
conn.execute(
|
||||
"INSERT INTO schema_meta(key, value) VALUES ('schema_version', ?)",
|
||||
(str(schema_version),),
|
||||
)
|
||||
conn.execute("INSERT INTO leases(lease_id, status) VALUES ('l1', 'active')")
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
def test_missing_database_is_down(self):
|
||||
probe = probe_control_plane_db(str(Path(self.tmp.name) / "absent.db"))
|
||||
self.assertEqual(probe.status, STATUS_DOWN)
|
||||
self.assertTrue(probe.required)
|
||||
self.assertIsNotNone(probe.latency_ms)
|
||||
|
||||
def test_matching_schema_is_ok(self):
|
||||
self._build_db(control_plane_db.SCHEMA_VERSION)
|
||||
probe = probe_control_plane_db(self.db_path)
|
||||
self.assertEqual(probe.status, STATUS_OK)
|
||||
self.assertEqual(
|
||||
probe.metadata["schema_version"], control_plane_db.SCHEMA_VERSION
|
||||
)
|
||||
self.assertEqual(probe.metadata["active_leases"], 1)
|
||||
|
||||
def test_mismatched_schema_is_degraded(self):
|
||||
self._build_db(control_plane_db.SCHEMA_VERSION + 99)
|
||||
probe = probe_control_plane_db(self.db_path)
|
||||
self.assertEqual(probe.status, STATUS_DEGRADED)
|
||||
|
||||
def test_probe_does_not_create_a_database(self):
|
||||
"""A health check must never initialise the substrate it inspects."""
|
||||
absent = str(Path(self.tmp.name) / "never-created.db")
|
||||
probe_control_plane_db(absent)
|
||||
self.assertFalse(Path(absent).exists())
|
||||
|
||||
def test_unreadable_database_is_down_not_raised(self):
|
||||
Path(self.db_path).write_text("this is not a sqlite database")
|
||||
probe = probe_control_plane_db(self.db_path)
|
||||
self.assertEqual(probe.status, STATUS_DOWN)
|
||||
|
||||
|
||||
class TestRedaction(unittest.TestCase):
|
||||
"""AC4 — redaction. No credential-shaped text crosses the boundary."""
|
||||
|
||||
def test_redacts_token_assignment(self):
|
||||
cleaned = redact("failed with token=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345")
|
||||
self.assertNotIn("ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345", cleaned)
|
||||
self.assertIn("[redacted]", cleaned)
|
||||
|
||||
def test_redacts_authorization_header_text(self):
|
||||
cleaned = redact("Authorization: Bearer abcdefghijklmnopqrstuvwxyz123456")
|
||||
self.assertNotIn("abcdefghijklmnopqrstuvwxyz123456", cleaned)
|
||||
|
||||
def test_redacts_long_opaque_strings(self):
|
||||
cleaned = redact("value 0123456789abcdef0123456789abcdef here")
|
||||
self.assertNotIn("0123456789abcdef0123456789abcdef", cleaned)
|
||||
|
||||
def test_url_userinfo_and_query_are_stripped(self):
|
||||
cleaned = redact_url("https://user:[email protected]/api/v1?token=xyz")
|
||||
self.assertNotIn("secretpass", cleaned)
|
||||
self.assertNotIn("token=xyz", cleaned)
|
||||
self.assertEqual(cleaned, "https://gitea.example.com/api/v1")
|
||||
|
||||
def test_url_inside_free_text_is_redacted(self):
|
||||
cleaned = redact("GET https://u:[email protected]/x?token=abc failed")
|
||||
self.assertNotIn("u:p@", cleaned)
|
||||
self.assertNotIn("token=abc", cleaned)
|
||||
|
||||
def test_gitea_probe_failure_detail_is_redacted(self):
|
||||
boom = RuntimeError(
|
||||
"connection refused for https://user:[email protected]/api/v1/version"
|
||||
)
|
||||
with mock.patch("webui.system_health.get_auth_header", return_value="token x"), \
|
||||
mock.patch("webui.system_health.api_request", side_effect=boom):
|
||||
probe = probe_gitea("gitea.example.com")
|
||||
self.assertEqual(probe.status, STATUS_DOWN)
|
||||
self.assertNotIn("hunter2", probe.detail)
|
||||
self.assertEqual(scan_text_for_client_secrets(probe.detail), [])
|
||||
|
||||
def test_credential_guard_refusal_is_a_status_not_a_crash(self):
|
||||
with mock.patch(
|
||||
"webui.system_health.get_auth_header",
|
||||
side_effect=RuntimeError("daemon guard refused"),
|
||||
):
|
||||
probe = probe_gitea("gitea.example.com")
|
||||
self.assertEqual(probe.status, STATUS_DEGRADED)
|
||||
self.assertFalse(probe.required)
|
||||
|
||||
|
||||
class TestNamespaceSummaries(unittest.TestCase):
|
||||
"""A web process cannot prove IDE namespace health, and must not claim to."""
|
||||
|
||||
def test_every_namespace_reports_unproven(self):
|
||||
rows = namespace_summaries()
|
||||
self.assertTrue(rows)
|
||||
for row in rows:
|
||||
with self.subTest(namespace=row["namespace"]):
|
||||
self.assertEqual(row["status"], "unproven")
|
||||
self.assertFalse(row["ide_namespace_proven"])
|
||||
self.assertIn("client_namespace", row["reason"])
|
||||
|
||||
|
||||
class TestSystemHealthRoutes(CleanParityMixin, unittest.TestCase):
|
||||
"""The HTTP surface: versioned path, status codes, read-only guard."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
clear_probe_cache()
|
||||
self.addCleanup(clear_probe_cache)
|
||||
self.client = TestClient(create_app())
|
||||
|
||||
def _patch_snapshot(self, probes, daemon_head="abc123"):
|
||||
snapshot = load_system_health(probes=probes, daemon_head=daemon_head)
|
||||
patcher = mock.patch(
|
||||
"webui.app.load_system_health",
|
||||
return_value=snapshot,
|
||||
)
|
||||
patcher.start()
|
||||
self.addCleanup(patcher.stop)
|
||||
return snapshot
|
||||
|
||||
def test_versioned_route_is_registered(self):
|
||||
self.assertEqual(API_PATH, "/api/v1/system/health")
|
||||
self._patch_snapshot(_ALL_HEALTHY)
|
||||
response = self.client.get(API_PATH)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
def test_healthy_payload_shape(self):
|
||||
self._patch_snapshot(_ALL_HEALTHY)
|
||||
data = self.client.get(API_PATH).json()
|
||||
self.assertEqual(data["status"], STATUS_OK)
|
||||
self.assertTrue(data["readiness"]["ready"])
|
||||
self.assertTrue(data["readiness"]["complete"])
|
||||
self.assertEqual(data["api"], API_PATH)
|
||||
self.assertEqual(len(data["dependencies"]), 3)
|
||||
for key in ("version", "process", "stale_runtime", "mcp_namespaces"):
|
||||
self.assertIn(key, data)
|
||||
self.assertIn("uptime_seconds", data["process"])
|
||||
self.assertIn("mutation_safe", data["stale_runtime"])
|
||||
|
||||
def test_degraded_dependency_returns_503(self):
|
||||
probes = (
|
||||
_probe("control_plane_db", STATUS_DOWN, detail="missing", kind="sqlite"),
|
||||
_probe("repository", STATUS_OK, kind="git"),
|
||||
)
|
||||
self._patch_snapshot(probes)
|
||||
response = self.client.get(API_PATH)
|
||||
self.assertEqual(response.status_code, 503)
|
||||
data = response.json()
|
||||
self.assertFalse(data["readiness"]["ready"])
|
||||
self.assertTrue(data["readiness"]["reasons"])
|
||||
|
||||
def test_dependency_entries_expose_status_and_latency(self):
|
||||
self._patch_snapshot(_ALL_HEALTHY)
|
||||
data = self.client.get(API_PATH).json()
|
||||
names = {entry["name"] for entry in data["dependencies"]}
|
||||
self.assertEqual(names, {"control_plane_db", "repository", "gitea"})
|
||||
for entry in data["dependencies"]:
|
||||
with self.subTest(dependency=entry["name"]):
|
||||
self.assertIn("status", entry)
|
||||
self.assertIn("required", entry)
|
||||
self.assertIn("latency_ms", entry)
|
||||
|
||||
def test_response_body_carries_no_client_secrets(self):
|
||||
self._patch_snapshot(_ALL_HEALTHY)
|
||||
body = self.client.get(API_PATH).text
|
||||
self.assertEqual(scan_text_for_client_secrets(body), [])
|
||||
|
||||
def test_deep_flag_is_forwarded(self):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc")
|
||||
with mock.patch(
|
||||
"webui.app.load_system_health", return_value=snapshot
|
||||
) as loader:
|
||||
self.client.get(f"{API_PATH}?deep=1")
|
||||
loader.assert_called_once_with(deep=True)
|
||||
|
||||
def test_shallow_is_the_default(self):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc")
|
||||
with mock.patch(
|
||||
"webui.app.load_system_health", return_value=snapshot
|
||||
) as loader:
|
||||
self.client.get(API_PATH)
|
||||
loader.assert_called_once_with(deep=False)
|
||||
|
||||
def test_route_rejects_mutation_methods(self):
|
||||
for method in ("POST", "PUT", "PATCH", "DELETE"):
|
||||
with self.subTest(method=method):
|
||||
response = self.client.request(method, API_PATH)
|
||||
self.assertEqual(response.status_code, 405)
|
||||
self.assertEqual(response.json()["error"], "read-only-mvp")
|
||||
|
||||
def test_default_shallow_call_skips_the_network_probe(self):
|
||||
"""The expensive probe must not run unless it was asked for."""
|
||||
with mock.patch("webui.system_health.probe_gitea") as probe:
|
||||
snapshot = load_system_health(deep=False)
|
||||
probe.assert_not_called()
|
||||
gitea = next(p for p in snapshot.dependencies if p.name == "gitea")
|
||||
self.assertEqual(gitea.status, STATUS_SKIPPED)
|
||||
|
||||
|
||||
class TestHealthRouteBackwardCompatibility(unittest.TestCase):
|
||||
"""`/health` is expanded additively; MVP consumers must keep working."""
|
||||
|
||||
def setUp(self):
|
||||
self.client = TestClient(create_app())
|
||||
|
||||
def test_mvp_keys_are_unchanged(self):
|
||||
data = self.client.get("/health").json()
|
||||
self.assertEqual(data["status"], "ok")
|
||||
self.assertEqual(data["service"], "mcp-control-plane-webui")
|
||||
self.assertEqual(data["mode"], "read-only-mvp")
|
||||
self.assertIn("timestamp", data)
|
||||
self.assertEqual(data["deployment"]["mode"], "internal-operator-console")
|
||||
|
||||
def test_health_points_at_the_versioned_api(self):
|
||||
data = self.client.get("/health").json()
|
||||
self.assertEqual(data["system_health_api"], API_PATH)
|
||||
self.assertIn("uptime_seconds", data)
|
||||
self.assertIn("started_at", data)
|
||||
|
||||
def test_health_runs_no_dependency_probe(self):
|
||||
"""Liveness must stay cheap: no probe, no snapshot assembly."""
|
||||
with mock.patch("webui.app.load_system_health") as loader:
|
||||
response = self.client.get("/health")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
loader.assert_not_called()
|
||||
|
||||
|
||||
class TestSnapshotSerialisation(CleanParityMixin, unittest.TestCase):
|
||||
def test_snapshot_dict_is_json_serialisable(self):
|
||||
snapshot = load_system_health(probes=_ALL_HEALTHY, daemon_head="abc123")
|
||||
encoded = json.dumps(snapshot_to_dict(snapshot))
|
||||
self.assertIn("readiness", encoded)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user