Fix Codex supervise MCP registration #248
@@ -3,7 +3,8 @@ act on them (approve / modify / reject).
|
||||
|
||||
Curses-based TUI; modify-then-approve shells out to $EDITOR. The
|
||||
approval handler wires to PRD 0016 (capability-block), which rebuilds
|
||||
the bottle Dockerfile. The egress-block tool was removed in issue #198.
|
||||
the bottle Dockerfile. Egress proposals are queued for operator review
|
||||
as full routes.yaml updates.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -40,6 +41,8 @@ from ..supervise import (
|
||||
STATUS_MODIFIED,
|
||||
STATUS_REJECTED,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_ALLOW,
|
||||
TOOL_EGRESS_BLOCK,
|
||||
archive_proposal,
|
||||
list_pending_proposals,
|
||||
render_diff,
|
||||
@@ -115,6 +118,8 @@ def _detail_lines(
|
||||
def _suffix_for_tool(tool: str) -> str:
|
||||
if tool == TOOL_CAPABILITY_BLOCK:
|
||||
return ".dockerfile"
|
||||
if tool in (TOOL_ALLOW, TOOL_EGRESS_BLOCK):
|
||||
return ".yaml"
|
||||
return ".txt"
|
||||
|
||||
|
||||
|
||||
+19
-12
@@ -5,7 +5,7 @@ queue/audit support. The sidecar (bot_bottle.supervise_server)
|
||||
sits on the bottle's internal network and exposes three MCP tools the
|
||||
agent calls when it hits a stuck-recovery category:
|
||||
|
||||
* egress-block — agent proposes a new routes.yaml
|
||||
* egress-block / allow — agent proposes a new routes.yaml
|
||||
* capability-block — agent proposes a new agent Dockerfile
|
||||
|
||||
Each tool call: the agent passes the full proposed file plus a
|
||||
@@ -49,27 +49,34 @@ SUPERVISE_HOSTNAME = "supervise"
|
||||
SUPERVISE_PORT = 9100
|
||||
|
||||
TOOL_CAPABILITY_BLOCK = "capability-block"
|
||||
TOOL_EGRESS_BLOCK = "egress-block"
|
||||
TOOL_ALLOW = "allow"
|
||||
TOOL_LIST_EGRESS_ROUTES = "list-egress-routes"
|
||||
TOOLS: tuple[str, ...] = (
|
||||
TOOL_ALLOW,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
TOOL_EGRESS_BLOCK,
|
||||
TOOL_LIST_EGRESS_ROUTES,
|
||||
)
|
||||
|
||||
# The supervise sidecar uses these to query egress's
|
||||
# introspection endpoint for the `list-egress-routes` MCP
|
||||
# tool. The hostname + port match egress's docker network
|
||||
# alias + listen port (see bot_bottle.egress.EGRESS_HOSTNAME
|
||||
# and backend.docker.egress.EGRESS_PORT — the values
|
||||
# are inlined here so the in-container supervise_server doesn't
|
||||
# need to import the egress package).
|
||||
EGRESS_FORWARD_PROXY = "http://egress:9099"
|
||||
# listen port (see backend.docker.egress.EGRESS_PORT). The supervise
|
||||
# daemon runs inside the sidecar bundle alongside egress, so loopback
|
||||
# is the stable address across docker, smolmachines, and Apple
|
||||
# Container backends.
|
||||
EGRESS_FORWARD_PROXY = "http://127.0.0.1:9099"
|
||||
EGRESS_INTROSPECT_URL = "http://_egress.local/allowlist"
|
||||
|
||||
# capability-block has no on-disk config the operator edits in place
|
||||
# (the Dockerfile is rebuilt, not patched), so it has no audit log
|
||||
# here — those changes are captured by git history + the rebuild
|
||||
# record laid down in PRD 0016. egress-block was removed in issue #198.
|
||||
COMPONENT_FOR_TOOL: dict[str, str] = {}
|
||||
# here — those changes are captured by git history + the rebuild record
|
||||
# laid down in PRD 0016.
|
||||
COMPONENT_FOR_TOOL: dict[str, str] = {
|
||||
TOOL_ALLOW: "egress",
|
||||
TOOL_EGRESS_BLOCK: "egress",
|
||||
}
|
||||
|
||||
STATUS_APPROVED = "approved"
|
||||
STATUS_MODIFIED = "modified"
|
||||
@@ -431,9 +438,9 @@ def sha256_hex(content: str) -> str:
|
||||
# Dockerfile and propose modifications.
|
||||
#
|
||||
# routes.yaml + allowlist used to live here too; PRD 0017 chunk 3
|
||||
# moved them behind the `list-egress-routes` MCP tool (live
|
||||
# state from egress's introspection endpoint) so the agent
|
||||
# always sees current data rather than a launch-time snapshot.
|
||||
# moved them behind the `list-egress-routes` MCP tool (live state
|
||||
# from egress's introspection endpoint) so the agent always sees
|
||||
# current data rather than a launch-time snapshot.
|
||||
CURRENT_CONFIG_DOCKERFILE = "Dockerfile"
|
||||
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
"""Supervise sidecar HTTP server (PRD 0013).
|
||||
|
||||
Per-bottle MCP server exposing tools the agent calls to propose config
|
||||
changes when stuck. The egress-block tool was removed in issue #198;
|
||||
the remaining tools are `capability-block` and `list-egress-routes`.
|
||||
changes when stuck. The tools are `allow`, `egress-block`,
|
||||
`capability-block`, and `list-egress-routes`.
|
||||
|
||||
Each queued tool call:
|
||||
|
||||
@@ -44,9 +44,15 @@ import urllib.request
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
# Same-directory import inside the bundle container; `supervise.py`
|
||||
# is COPYed alongside this file by Dockerfile.sidecars.
|
||||
import supervise as _sv
|
||||
try:
|
||||
# Same-directory imports inside the bundle container; these files are
|
||||
# COPYed flat under /app by Dockerfile.sidecars.
|
||||
from egress_addon_core import load_routes
|
||||
import supervise as _sv
|
||||
except ModuleNotFoundError:
|
||||
# Package imports for host-side tests and tooling.
|
||||
from .egress_addon_core import load_routes
|
||||
from . import supervise as _sv
|
||||
|
||||
|
||||
# --- JSON-RPC / MCP plumbing ----------------------------------------------
|
||||
@@ -142,8 +148,9 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [
|
||||
"allowlist. Returns JSON with one entry per allowed host, "
|
||||
"each carrying its matches rules (if any) and whether "
|
||||
"the proxy injects Authorization for the route. Use this "
|
||||
"before composing an `egress-block` proposal so the new "
|
||||
"routes file extends the live one rather than replacing it."
|
||||
"before composing an `allow` or `egress-block` proposal so "
|
||||
"the new routes file extends the live one rather than "
|
||||
"replacing it."
|
||||
),
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
@@ -151,6 +158,54 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [
|
||||
"additionalProperties": False,
|
||||
},
|
||||
},
|
||||
{
|
||||
"name": _sv.TOOL_ALLOW,
|
||||
"description": (
|
||||
"Request operator approval to change the bottle's egress "
|
||||
"allowlist. Pass the full proposed routes.yaml content, not "
|
||||
"just the new host, plus a justification. Use "
|
||||
"`list-egress-routes` first so the proposal preserves existing "
|
||||
"routes."
|
||||
),
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"routes_yaml": {
|
||||
"type": "string",
|
||||
"description": "Full proposed /etc/egress/routes.yaml content.",
|
||||
},
|
||||
"justification": {
|
||||
"type": "string",
|
||||
"description": "Why this egress route is needed.",
|
||||
},
|
||||
},
|
||||
"required": ["routes_yaml", "justification"],
|
||||
},
|
||||
},
|
||||
{
|
||||
"name": _sv.TOOL_EGRESS_BLOCK,
|
||||
"description": (
|
||||
"Request operator approval to change the bottle's egress "
|
||||
"allowlist after a blocked outbound request. Pass the full "
|
||||
"proposed routes.yaml content plus a justification. Use "
|
||||
"`list-egress-routes` first so the proposal preserves existing "
|
||||
"routes."
|
||||
),
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"routes_yaml": {
|
||||
"type": "string",
|
||||
"description": "Full proposed /etc/egress/routes.yaml content.",
|
||||
},
|
||||
"justification": {
|
||||
"type": "string",
|
||||
"description": "Why this egress route is needed.",
|
||||
},
|
||||
},
|
||||
"required": ["routes_yaml", "justification"],
|
||||
},
|
||||
},
|
||||
{
|
||||
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||
"description": (
|
||||
@@ -182,11 +237,12 @@ TOOL_DEFINITIONS: list[dict[str, object]] = [
|
||||
]
|
||||
|
||||
|
||||
# Map each non-egress tool to the input field that carries the agent's
|
||||
# payload (stored in Proposal.proposed_file). egress-block builds its
|
||||
# payload from structured input fields in `handle_egress_block`.
|
||||
# Map each proposal tool to the input field that carries the agent's
|
||||
# payload (stored in Proposal.proposed_file).
|
||||
PROPOSED_FILE_FIELD: dict[str, str] = {
|
||||
_sv.TOOL_ALLOW: "routes_yaml",
|
||||
_sv.TOOL_CAPABILITY_BLOCK: "dockerfile",
|
||||
_sv.TOOL_EGRESS_BLOCK: "routes_yaml",
|
||||
}
|
||||
|
||||
|
||||
@@ -203,6 +259,14 @@ def validate_proposed_file(tool: str, content: str) -> None:
|
||||
# Dockerfiles are too varied to validate syntactically beyond
|
||||
# non-empty. The operator reads the diff in the TUI.
|
||||
pass
|
||||
elif tool in (_sv.TOOL_ALLOW, _sv.TOOL_EGRESS_BLOCK):
|
||||
try:
|
||||
load_routes(content)
|
||||
except ValueError as e:
|
||||
raise _RpcError(
|
||||
ERR_INVALID_PARAMS,
|
||||
f"{tool}: proposed routes.yaml is not valid: {e}",
|
||||
) from e
|
||||
else:
|
||||
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
|
||||
|
||||
|
||||
@@ -317,15 +317,22 @@ class TestToolConstants(unittest.TestCase):
|
||||
def test_tools_tuple_matches_individual_constants(self):
|
||||
self.assertEqual(
|
||||
(
|
||||
supervise.TOOL_ALLOW,
|
||||
TOOL_CAPABILITY_BLOCK,
|
||||
supervise.TOOL_EGRESS_BLOCK,
|
||||
supervise.TOOL_LIST_EGRESS_ROUTES,
|
||||
),
|
||||
supervise.TOOLS,
|
||||
)
|
||||
|
||||
def test_component_map_has_no_entries(self):
|
||||
# egress-block removed in issue #198; capability-block never had one.
|
||||
self.assertEqual({}, supervise.COMPONENT_FOR_TOOL)
|
||||
def test_component_map_has_egress_entries(self):
|
||||
self.assertEqual(
|
||||
{
|
||||
supervise.TOOL_ALLOW: "egress",
|
||||
supervise.TOOL_EGRESS_BLOCK: "egress",
|
||||
},
|
||||
supervise.COMPONENT_FOR_TOOL,
|
||||
)
|
||||
|
||||
|
||||
class _StubSupervise(supervise.Supervise):
|
||||
|
||||
@@ -2,9 +2,6 @@
|
||||
|
||||
The curses TUI itself isn't exercised here — these tests cover the
|
||||
discovery + approve/reject paths that the TUI's key handlers call into.
|
||||
|
||||
egress-block (add_route) was removed in issue #198; the TestEgressApplyWiring
|
||||
class and all stubs for add_route have been dropped accordingly.
|
||||
"""
|
||||
|
||||
import os
|
||||
@@ -33,6 +30,8 @@ FIXED = datetime(2026, 5, 25, 12, 0, 0, tzinfo=timezone.utc)
|
||||
def _proposal(slug: str = "dev", tool: str = TOOL_CAPABILITY_BLOCK) -> Proposal:
|
||||
payloads = {
|
||||
TOOL_CAPABILITY_BLOCK: "FROM python:3.13\n",
|
||||
supervise.TOOL_ALLOW: "routes:\n - host: example.com\n",
|
||||
supervise.TOOL_EGRESS_BLOCK: "routes:\n - host: example.com\n",
|
||||
}
|
||||
payload = payloads.get(tool, "")
|
||||
return Proposal.new(
|
||||
@@ -154,6 +153,14 @@ class TestApproveReject(_FakeHomeMixin, unittest.TestCase):
|
||||
supervise_cli.approve(qp)
|
||||
self.assertEqual([], read_audit_entries("egress", "dev"))
|
||||
|
||||
def test_approve_egress_block_writes_audit_log(self):
|
||||
qp = self._enqueue(tool=supervise.TOOL_EGRESS_BLOCK)
|
||||
supervise_cli.approve(qp)
|
||||
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)
|
||||
|
||||
|
||||
# class TestCapabilityApplyWiring(_FakeHomeMixin, unittest.TestCase):
|
||||
# # DISABLED — capability_apply functionality is currently commented out.
|
||||
|
||||
@@ -54,13 +54,19 @@ class TestValidation(unittest.TestCase):
|
||||
)
|
||||
|
||||
def test_empty_proposed_file_rejected_for_tools_with_file_field(self):
|
||||
# egress-block has structured input (validated in
|
||||
# _validate_and_bundle_egress_route, not here) and
|
||||
# list-egress-routes takes no input. Only capability-block
|
||||
# goes through `validate_proposed_file`.
|
||||
with self.assertRaises(_RpcError):
|
||||
validate_proposed_file(_sv.TOOL_CAPABILITY_BLOCK, " \n\t")
|
||||
|
||||
def test_egress_routes_yaml_is_validated(self):
|
||||
validate_proposed_file(
|
||||
_sv.TOOL_ALLOW,
|
||||
"routes:\n - host: example.com\n",
|
||||
)
|
||||
|
||||
def test_invalid_egress_routes_yaml_rejected(self):
|
||||
with self.assertRaises(_RpcError):
|
||||
validate_proposed_file(_sv.TOOL_EGRESS_BLOCK, "routes: nope\n")
|
||||
|
||||
|
||||
# --- JSON-RPC parsing ------------------------------------------------------
|
||||
|
||||
@@ -141,7 +147,9 @@ class TestHandleToolsList(unittest.TestCase):
|
||||
names = [t["name"] for t in result["tools"]] # type: ignore[index]
|
||||
self.assertEqual(
|
||||
sorted([
|
||||
_sv.TOOL_ALLOW,
|
||||
_sv.TOOL_CAPABILITY_BLOCK,
|
||||
_sv.TOOL_EGRESS_BLOCK,
|
||||
_sv.TOOL_LIST_EGRESS_ROUTES,
|
||||
]),
|
||||
sorted(names),
|
||||
@@ -172,6 +180,17 @@ class TestHandleToolsList(unittest.TestCase):
|
||||
# No `required` array because no inputs are required.
|
||||
self.assertNotIn("required", schema) # type: ignore[operator]
|
||||
|
||||
def test_egress_tools_take_routes_yaml_and_justification(self):
|
||||
for tool_name in (_sv.TOOL_ALLOW, _sv.TOOL_EGRESS_BLOCK):
|
||||
with self.subTest(tool_name=tool_name):
|
||||
tool = next(t for t in TOOL_DEFINITIONS if t["name"] == tool_name)
|
||||
schema = tool["inputSchema"]
|
||||
self.assertEqual("object", schema["type"]) # type: ignore[index]
|
||||
self.assertEqual(
|
||||
["routes_yaml", "justification"],
|
||||
schema["required"], # type: ignore[index]
|
||||
)
|
||||
|
||||
|
||||
class TestHandleToolsCall(unittest.TestCase):
|
||||
def setUp(self):
|
||||
@@ -220,6 +239,26 @@ class TestHandleToolsCall(unittest.TestCase):
|
||||
self.assertIn("status: approved", text)
|
||||
self.assertIn("notes: lgtm", text)
|
||||
|
||||
def test_allow_round_trips_through_queue(self):
|
||||
responder = self._respond_when_proposal_appears(_sv.STATUS_APPROVED, notes="ok")
|
||||
try:
|
||||
result = handle_tools_call(
|
||||
{
|
||||
"name": _sv.TOOL_ALLOW,
|
||||
"arguments": {
|
||||
"routes_yaml": "routes:\n - host: example.com\n",
|
||||
"justification": "need example.com",
|
||||
},
|
||||
},
|
||||
self.config,
|
||||
)
|
||||
finally:
|
||||
responder.join()
|
||||
self.assertFalse(result["isError"]) # type: ignore[index]
|
||||
text = result["content"][0]["text"] # type: ignore[index]
|
||||
self.assertIn("status: approved", text)
|
||||
self.assertIn("notes: ok", text)
|
||||
|
||||
def test_rejected_response_sets_isError(self):
|
||||
responder = self._respond_when_proposal_appears(_sv.STATUS_REJECTED, notes="nope")
|
||||
try:
|
||||
@@ -412,7 +451,8 @@ class TestHttpEndToEnd(unittest.TestCase):
|
||||
self.assertEqual(1, result["id"])
|
||||
names = [t["name"] for t in result["result"]["tools"]] # type: ignore[index]
|
||||
self.assertIn(_sv.TOOL_CAPABILITY_BLOCK, names)
|
||||
self.assertNotIn("egress-block", names)
|
||||
self.assertIn(_sv.TOOL_ALLOW, names)
|
||||
self.assertIn(_sv.TOOL_EGRESS_BLOCK, names)
|
||||
|
||||
def test_unknown_method_returns_jsonrpc_error(self):
|
||||
result = self._post_jsonrpc(
|
||||
|
||||
Reference in New Issue
Block a user