feat(supervise): host TUI drives approvals over HTTP, not the DB (Step 2b/2c)
The `bot-bottle supervise` operator TUI read and wrote the queue DB directly and tried a backend-specific live "apply" (which was unwired — it raised). It now talks only to the orchestrator control plane: - OrchestratorClient gains supervise_pending() + supervise_respond(). - discover_orchestrator_url() finds the one running per-host control plane by health-probing the backends' well-known :8099 addresses (docker publishes on loopback; the firecracker infra VM serves it on the orchestrator TAP) — no backend branching in the TUI. - discover_pending/approve/reject call the client; the server does the apply + response + audit atomically. The dead direct-DB apply/audit helpers and the docker/macos applicator imports are gone. - A missing control plane is now a clean one-line error up front, not a mid-curses crash. CLI tests move to mocking the client (the DB-write behaviour they used to assert is now server-side, covered by test_orchestrator_service). Docker's orchestrator-container DB wiring lands next so its /supervise endpoints hit the same shared DB. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
This commit is contained in:
@@ -1,31 +1,24 @@
|
||||
"""Unit: supervise headless paths (PRD 0013 phase 4, PRD 0016).
|
||||
"""Unit: supervise headless paths — the discovery + approve/reject that the
|
||||
TUI key handlers call into.
|
||||
|
||||
The curses TUI itself isn't exercised here — these tests cover the
|
||||
discovery + approve/reject paths that the TUI's key handlers call into.
|
||||
These go through the orchestrator HTTP client now (the operator never
|
||||
touches the DB directly), so the client is mocked here; the server-side
|
||||
apply / response / audit is covered in test_orchestrator_service.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from bot_bottle import supervise
|
||||
from tests.unit import use_bottle_root
|
||||
from bot_bottle.audit_store import AuditStore
|
||||
from bot_bottle.cli import supervise as supervise_cli
|
||||
from bot_bottle.queue_store import QueueStore
|
||||
from bot_bottle.supervise import (
|
||||
Proposal,
|
||||
STATUS_APPROVED,
|
||||
STATUS_MODIFIED,
|
||||
STATUS_REJECTED,
|
||||
TOOL_EGRESS_ALLOW,
|
||||
TOOL_EGRESS_BLOCK,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
TOOL_EGRESS_TOKEN_ALLOW,
|
||||
read_audit_entries,
|
||||
read_response,
|
||||
sha256_hex,
|
||||
)
|
||||
|
||||
@@ -33,198 +26,114 @@ from bot_bottle.supervise import (
|
||||
FIXED = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc)
|
||||
|
||||
|
||||
def _proposal(slug: str = "dev", tool: str = TOOL_EGRESS_ALLOW) -> Proposal:
|
||||
def _proposal(slug: str = "dev", tool: str = TOOL_EGRESS_ALLOW,
|
||||
*, now: datetime = FIXED) -> Proposal:
|
||||
payloads = {
|
||||
supervise.TOOL_EGRESS_ALLOW: "routes:\n - host: example.com\n",
|
||||
supervise.TOOL_EGRESS_BLOCK: "routes:\n - host: example.com\n",
|
||||
TOOL_EGRESS_ALLOW: "routes:\n - host: example.com\n",
|
||||
TOOL_EGRESS_BLOCK: "routes:\n - host: example.com\n",
|
||||
TOOL_GITLEAKS_ALLOW: "file: tests/test_fixture.py\nline: 3\n",
|
||||
TOOL_EGRESS_TOKEN_ALLOW: "host: api.example.com\ndetector: token\n",
|
||||
}
|
||||
payload = payloads.get(tool, "")
|
||||
return Proposal.new(
|
||||
bottle_slug=slug, tool=tool,
|
||||
proposed_file=payload,
|
||||
justification=f"needed for {slug}",
|
||||
current_file_hash=sha256_hex(payload),
|
||||
now=FIXED,
|
||||
bottle_slug=slug, tool=tool, proposed_file=payload,
|
||||
justification=f"needed for {slug}", current_file_hash=sha256_hex(payload),
|
||||
now=now,
|
||||
)
|
||||
|
||||
|
||||
class _FakeHomeMixin:
|
||||
"""Point bot_bottle_root at a temp dir (via BOT_BOTTLE_ROOT) for the test."""
|
||||
class _ClientMixin:
|
||||
"""Install a mock orchestrator client as the CLI-session singleton."""
|
||||
|
||||
def _setup_fake_home(self):
|
||||
self._tmp = tempfile.TemporaryDirectory(prefix="supervise-test.")
|
||||
self._restore_home = use_bottle_root(Path(self._tmp.name) / ".bot-bottle")
|
||||
QueueStore("").migrate()
|
||||
AuditStore().migrate()
|
||||
|
||||
def _teardown_fake_home(self):
|
||||
self._restore_home()
|
||||
self._tmp.cleanup()
|
||||
def _install_client(self, pending: "list[Proposal] | None" = None) -> MagicMock:
|
||||
client = MagicMock()
|
||||
client.supervise_pending.return_value = [
|
||||
p.to_dict() for p in (pending or [])
|
||||
]
|
||||
patcher = patch.object(supervise_cli, "_client", return_value=client)
|
||||
patcher.start()
|
||||
self.addCleanup(patcher.stop) # type: ignore[attr-defined]
|
||||
self.addCleanup( # type: ignore[attr-defined]
|
||||
lambda: setattr(supervise_cli, "_client_instance", None))
|
||||
return client
|
||||
|
||||
|
||||
class TestDiscoverPending(_FakeHomeMixin, unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
|
||||
def tearDown(self):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def test_empty_when_no_queues(self):
|
||||
class TestDiscoverPending(_ClientMixin, unittest.TestCase):
|
||||
def test_empty(self) -> None:
|
||||
self._install_client([])
|
||||
self.assertEqual([], supervise_cli.discover_pending())
|
||||
|
||||
def test_walks_all_slug_subdirs(self):
|
||||
for slug in ("dev", "api"):
|
||||
supervise.write_proposal(_proposal(slug=slug))
|
||||
def test_lists_all_bottles(self) -> None:
|
||||
self._install_client([_proposal("dev"), _proposal("api")])
|
||||
pending = supervise_cli.discover_pending()
|
||||
self.assertEqual({"dev", "api"}, {qp.proposal.bottle_slug for qp in pending})
|
||||
self.assertEqual(
|
||||
{"dev", "api"}, {qp.proposal.bottle_slug for qp in pending})
|
||||
|
||||
def test_sorted_by_arrival_across_bottles(self):
|
||||
early = Proposal.new(
|
||||
bottle_slug="api", tool=TOOL_EGRESS_ALLOW,
|
||||
proposed_file="routes:\n - host: early.example.com\n", justification="early",
|
||||
current_file_hash="h",
|
||||
now=datetime(2026, 5, 25, 10, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
late = Proposal.new(
|
||||
bottle_slug="dev", tool=TOOL_EGRESS_ALLOW,
|
||||
proposed_file="routes:\n - host: late.example.com\n", justification="late",
|
||||
current_file_hash="h",
|
||||
now=datetime(2026, 5, 25, 14, 0, 0, tzinfo=timezone.utc),
|
||||
)
|
||||
for p in (late, early):
|
||||
supervise.write_proposal(p)
|
||||
def test_sorted_by_arrival(self) -> None:
|
||||
early = _proposal(
|
||||
"api", now=datetime(2026, 5, 25, 10, 0, 0, tzinfo=timezone.utc))
|
||||
late = _proposal(
|
||||
"dev", now=datetime(2026, 5, 25, 14, 0, 0, tzinfo=timezone.utc))
|
||||
self._install_client([late, early])
|
||||
pending = supervise_cli.discover_pending()
|
||||
self.assertEqual([early.id, late.id], [qp.proposal.id for qp in pending])
|
||||
|
||||
def test_excludes_already_responded(self):
|
||||
p = _proposal()
|
||||
supervise.write_proposal(p)
|
||||
supervise.write_response("dev", supervise.Response(
|
||||
proposal_id=p.id, status=STATUS_APPROVED, notes="",
|
||||
))
|
||||
self.assertEqual([], supervise_cli.discover_pending())
|
||||
|
||||
class TestApproveReject(_ClientMixin, unittest.TestCase):
|
||||
def _qp(self, tool: str = TOOL_EGRESS_ALLOW) -> "supervise_cli.QueuedProposal":
|
||||
return supervise_cli.QueuedProposal(proposal=_proposal(tool=tool))
|
||||
|
||||
class TestApproveReject(_FakeHomeMixin, unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._setup_fake_home()
|
||||
|
||||
def tearDown(self):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def _enqueue(self, tool: str = TOOL_EGRESS_ALLOW):
|
||||
p = _proposal(tool=tool)
|
||||
supervise.write_proposal(p)
|
||||
return supervise_cli.QueuedProposal(proposal=p)
|
||||
|
||||
def test_approve_writes_response(self):
|
||||
qp = self._enqueue()
|
||||
with patch(
|
||||
"bot_bottle.cli.supervise.apply_routes_change",
|
||||
return_value=("routes: []\n", "routes:\n - host: example.com\n"),
|
||||
):
|
||||
supervise_cli.approve(qp)
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual(STATUS_APPROVED, resp.status)
|
||||
self.assertIsNone(resp.final_file)
|
||||
|
||||
def test_approve_with_final_file_marks_modified(self):
|
||||
qp = self._enqueue()
|
||||
with patch(
|
||||
"bot_bottle.cli.supervise.apply_routes_change",
|
||||
return_value=("routes: []\n", "routes:\n - host: edited.example.com\n"),
|
||||
):
|
||||
supervise_cli.approve(
|
||||
qp,
|
||||
final_file="routes:\n - host: edited.example.com\n",
|
||||
notes="tweaked",
|
||||
)
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual(STATUS_MODIFIED, resp.status)
|
||||
self.assertEqual("routes:\n - host: edited.example.com\n", resp.final_file)
|
||||
self.assertEqual("tweaked", resp.notes)
|
||||
|
||||
def test_reject_writes_rejection(self):
|
||||
qp = self._enqueue()
|
||||
supervise_cli.reject(qp, reason="nope")
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual(STATUS_REJECTED, resp.status)
|
||||
self.assertEqual("nope", resp.notes)
|
||||
|
||||
def test_approve_egress_block_writes_audit_log(self):
|
||||
qp = self._enqueue(tool=supervise.TOOL_EGRESS_BLOCK)
|
||||
with patch(
|
||||
"bot_bottle.cli.supervise.apply_routes_change",
|
||||
return_value=("routes: []\n", "routes:\n - host: example.com\n"),
|
||||
) as apply_routes_change:
|
||||
supervise_cli.approve(qp)
|
||||
apply_routes_change.assert_called_once_with(
|
||||
"dev",
|
||||
"routes:\n - host: example.com\n",
|
||||
def test_approve_calls_respond(self) -> None:
|
||||
client = self._install_client()
|
||||
qp = self._qp()
|
||||
supervise_cli.approve(qp)
|
||||
client.supervise_respond.assert_called_once_with(
|
||||
qp.proposal.id, bottle_slug="dev", decision="approve",
|
||||
notes="", final_file=None,
|
||||
)
|
||||
entries = read_audit_entries("egress", "dev")
|
||||
self.assertEqual(1, len(entries))
|
||||
self.assertEqual(STATUS_APPROVED, entries[0].operator_action)
|
||||
self.assertEqual("needed for dev", entries[0].justification)
|
||||
|
||||
def test_approve_gitleaks_allow_leaves_response_for_gate(self):
|
||||
qp = self._enqueue(tool=TOOL_GITLEAKS_ALLOW)
|
||||
supervise_cli.approve(qp, notes="dummy fixture")
|
||||
# Gate polls the DB for the response; TUI must not archive it.
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual(STATUS_APPROVED, resp.status)
|
||||
self.assertEqual("dummy fixture", resp.notes)
|
||||
def test_modify_sets_decision_and_final_file(self) -> None:
|
||||
client = self._install_client()
|
||||
qp = self._qp()
|
||||
edited = "routes:\n - host: edited.example.com\n"
|
||||
supervise_cli.approve(qp, final_file=edited, notes="tweaked")
|
||||
client.supervise_respond.assert_called_once_with(
|
||||
qp.proposal.id, bottle_slug="dev", decision="modify",
|
||||
notes="tweaked", final_file=edited,
|
||||
)
|
||||
|
||||
def test_tui_gitleaks_allow_requires_reason(self):
|
||||
qp = self._enqueue(tool=TOOL_GITLEAKS_ALLOW)
|
||||
def test_reject_calls_respond(self) -> None:
|
||||
client = self._install_client()
|
||||
qp = self._qp()
|
||||
supervise_cli.reject(qp, reason="nope")
|
||||
client.supervise_respond.assert_called_once_with(
|
||||
qp.proposal.id, bottle_slug="dev", decision="reject", notes="nope",
|
||||
)
|
||||
|
||||
def test_tui_report_only_requires_reason(self) -> None:
|
||||
self._install_client()
|
||||
qp = self._qp(tool=TOOL_GITLEAKS_ALLOW)
|
||||
with patch.object(supervise_cli, "_prompt", return_value=""):
|
||||
status = supervise_cli._approve_from_tui(None, qp) # type: ignore[arg-type]
|
||||
self.assertEqual("approve aborted (empty reason)", status)
|
||||
|
||||
def test_tui_gitleaks_allow_writes_reason(self):
|
||||
qp = self._enqueue(tool=TOOL_GITLEAKS_ALLOW)
|
||||
def test_tui_report_only_writes_reason(self) -> None:
|
||||
client = self._install_client()
|
||||
qp = self._qp(tool=TOOL_GITLEAKS_ALLOW)
|
||||
with patch.object(supervise_cli, "_prompt", return_value="test fixture"):
|
||||
status = supervise_cli._approve_from_tui(None, qp) # type: ignore[arg-type]
|
||||
self.assertIn("approved gitleaks-allow", status)
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual("test fixture", resp.notes)
|
||||
client.supervise_respond.assert_called_once()
|
||||
self.assertEqual(
|
||||
"test fixture", client.supervise_respond.call_args.kwargs["notes"])
|
||||
|
||||
def test_approve_token_allow_leaves_response_for_egress(self):
|
||||
qp = self._enqueue(tool=TOOL_EGRESS_TOKEN_ALLOW)
|
||||
supervise_cli.approve(qp, notes="false positive")
|
||||
# The egress addon polls the DB for the response; the TUI must
|
||||
# not archive it (the addon archives after reading).
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual(STATUS_APPROVED, resp.status)
|
||||
self.assertEqual("false positive", resp.notes)
|
||||
|
||||
def test_token_allow_writes_no_audit_log(self):
|
||||
qp = self._enqueue(tool=TOOL_EGRESS_TOKEN_ALLOW)
|
||||
supervise_cli.approve(qp, notes="false positive")
|
||||
self.assertEqual([], read_audit_entries("egress", "dev"))
|
||||
|
||||
def test_tui_token_allow_requires_reason(self):
|
||||
qp = self._enqueue(tool=TOOL_EGRESS_TOKEN_ALLOW)
|
||||
with patch.object(supervise_cli, "_prompt", return_value=""):
|
||||
status = supervise_cli._approve_from_tui(None, qp) # type: ignore[arg-type]
|
||||
self.assertEqual("approve aborted (empty reason)", status)
|
||||
|
||||
def test_tui_token_allow_writes_reason(self):
|
||||
qp = self._enqueue(tool=TOOL_EGRESS_TOKEN_ALLOW)
|
||||
with patch.object(supervise_cli, "_prompt", return_value="legit"):
|
||||
status = supervise_cli._approve_from_tui(None, qp) # type: ignore[arg-type]
|
||||
self.assertIn("approved egress-token-allow", status)
|
||||
resp = read_response(qp.proposal.bottle_slug, qp.proposal.id)
|
||||
self.assertEqual("legit", resp.notes)
|
||||
|
||||
def test_suffix_for_token_allow_is_txt(self):
|
||||
self.assertEqual(".txt", supervise_cli._suffix_for_tool(TOOL_EGRESS_TOKEN_ALLOW))
|
||||
def test_suffix_for_token_allow_is_txt(self) -> None:
|
||||
self.assertEqual(
|
||||
".txt", supervise_cli._suffix_for_tool(TOOL_EGRESS_TOKEN_ALLOW))
|
||||
|
||||
|
||||
class TestEditInEditor(unittest.TestCase):
|
||||
def test_runs_editor_returns_edited_content(self):
|
||||
def test_runs_editor_returns_edited_content(self) -> None:
|
||||
original_editor = os.environ.get("EDITOR")
|
||||
try:
|
||||
with tempfile.NamedTemporaryFile(
|
||||
@@ -245,7 +154,7 @@ class TestEditInEditor(unittest.TestCase):
|
||||
else:
|
||||
os.environ["EDITOR"] = original_editor
|
||||
|
||||
def test_returns_none_when_unchanged(self):
|
||||
def test_returns_none_when_unchanged(self) -> None:
|
||||
original_editor = os.environ.get("EDITOR")
|
||||
try:
|
||||
with tempfile.NamedTemporaryFile(
|
||||
|
||||
Reference in New Issue
Block a user