Fix Codex supervise MCP registration #248

Merged
didericis merged 12 commits from fix-codex-mcp-supervise-transport into main 2026-06-23 16:42:20 -04:00
3 changed files with 103 additions and 83 deletions
Showing only changes of commit 77bdaf0a96 - Show all commits
+28 -47
View File
@@ -9,19 +9,13 @@ from __future__ import annotations
import os
import subprocess
from pathlib import Path
from ...bottle_state import egress_state_dir
from ...egress import EGRESS_ROUTES_FILENAME, EGRESS_ROUTES_IN_CONTAINER
from ...egress_addon_core import load_routes
from ...egress import EGRESS_ROUTES_IN_CONTAINER
from ...log import warn
from ..egress_apply import EgressApplicator, EgressApplyError
from .sidecar_bundle import sidecar_bundle_container_name
class EgressApplyError(RuntimeError):
pass
def fetch_current_routes(slug: str) -> str:
container = sidecar_bundle_container_name(slug)
r = subprocess.run(
@@ -32,54 +26,41 @@ def fetch_current_routes(slug: str) -> str:
raise EgressApplyError(
f"could not read routes.yaml from {container}: "
f"{(r.stderr or '').strip() or 'container not running?'}"
)
)
return r.stdout
class DockerEgressApplicator(EgressApplicator):
def _signal_bundle_reload(self, slug: str) -> None:
container = sidecar_bundle_container_name(slug)
result = subprocess.run(
["docker", "kill", "--signal", "HUP", container],
capture_output=True, text=True, check=False, env=os.environ,
)
if result.returncode != 0:
last_error = (result.stderr or "").strip() or (result.stdout or "").strip()
warn(
f"egress: routes updated on disk for {slug}, but bundle reload failed: "
f"{last_error or 'docker kill failed'}"
)
raise EgressApplyError(
f"could not reload egress bundle {container}: "
f"{last_error or 'docker kill failed'}"
)
_applicator = DockerEgressApplicator()
def apply_routes_change(slug: str, content: str) -> tuple[str, str]:
"""Persist `content` to the live routes file and reload egress."""
validate_routes_content(content)
routes_path = _routes_path(slug)
routes_path.parent.mkdir(parents=True, exist_ok=True)
before = routes_path.read_text(encoding="utf-8") if routes_path.exists() else ""
routes_path.write_text(content, encoding="utf-8")
routes_path.chmod(0o600)
_signal_bundle_reload(slug)
return before, content
return _applicator.apply_routes_change(slug, content)
def validate_routes_content(content: str) -> None:
try:
load_routes(content)
except ValueError as e:
raise EgressApplyError(
f"proposed routes.yaml is not valid: {e}"
) from e
def _routes_path(slug: str) -> Path:
return egress_state_dir(slug) / EGRESS_ROUTES_FILENAME
def _signal_bundle_reload(slug: str) -> None:
container = sidecar_bundle_container_name(slug)
result = subprocess.run(
["docker", "kill", "--signal", "HUP", container],
capture_output=True, text=True, check=False, env=os.environ,
)
if result.returncode != 0:
last_error = (result.stderr or "").strip() or (result.stdout or "").strip()
warn(
f"egress: routes updated on disk for {slug}, but bundle reload failed: "
f"{last_error or 'docker kill failed'}"
)
raise EgressApplyError(
f"could not reload egress bundle {container}: "
f"{last_error or 'docker kill failed'}"
)
validate_routes_content = EgressApplicator.validate_routes_content
__all__ = [
"DockerEgressApplicator",
didericis marked this conversation as resolved Outdated
Outdated
Review

No need to preserve legacy behavior

No need to preserve legacy behavior
"EgressApplyError",
"apply_routes_change",
"fetch_current_routes",
+50
View File
@@ -0,0 +1,50 @@
"""Shared base class for host-side egress apply across backends.
Each backend subclasses EgressApplicator and overrides _signal_bundle_reload
with the backend-specific kill command.
"""
from __future__ import annotations
from abc import ABC, abstractmethod
from pathlib import Path
from ..bottle_state import egress_state_dir
from ..egress import EGRESS_ROUTES_FILENAME
from ..egress_addon_core import load_routes
class EgressApplyError(RuntimeError):
pass
class EgressApplicator(ABC):
def apply_routes_change(self, slug: str, content: str) -> tuple[str, str]:
"""Persist `content` to the live routes file and reload egress."""
self.validate_routes_content(content)
routes_path = self._routes_path(slug)
routes_path.parent.mkdir(parents=True, exist_ok=True)
before = routes_path.read_text(encoding="utf-8") if routes_path.exists() else ""
routes_path.write_text(content, encoding="utf-8")
routes_path.chmod(0o600)
self._signal_bundle_reload(slug)
return before, content
@staticmethod
def validate_routes_content(content: str) -> None:
try:
load_routes(content)
except ValueError as e:
raise EgressApplyError(
f"proposed routes.yaml is not valid: {e}"
) from e
@staticmethod
def _routes_path(slug: str) -> Path:
return egress_state_dir(slug) / EGRESS_ROUTES_FILENAME
@abstractmethod
def _signal_bundle_reload(self, slug: str) -> None: ...
__all__ = ["EgressApplicator", "EgressApplyError"]
@@ -8,47 +8,36 @@ from __future__ import annotations
import os
import subprocess
from pathlib import Path
from ...bottle_state import egress_state_dir
from ...egress import EGRESS_ROUTES_FILENAME
from ...log import warn
from ..docker.egress_apply import EgressApplyError, validate_routes_content
from ..egress_apply import EgressApplicator, EgressApplyError
from .launch import sidecar_container_name
class MacOSContainerEgressApplicator(EgressApplicator):
def _signal_bundle_reload(self, slug: str) -> None:
container = sidecar_container_name(slug)
result = subprocess.run(
didericis marked this conversation as resolved Outdated
Outdated
Review

This belongs in a base class shared between backends, named something like "EgressApplicator", and calls an internal/overridden _signal_bundle_reload function that differs by backend (seems to be the only difference between backends)

This belongs in a base class shared between backends, named something like "EgressApplicator", and calls an internal/overridden `_signal_bundle_reload` function that differs by backend (seems to be the only difference between backends)
["container", "kill", "--signal", "HUP", container],
capture_output=True, text=True, check=False, env=os.environ,
)
if result.returncode != 0:
last_error = (result.stderr or "").strip() or (result.stdout or "").strip()
warn(
f"egress: routes updated on disk for {slug}, but bundle reload failed: "
f"{last_error or 'container kill failed'}"
)
raise EgressApplyError(
f"could not reload egress bundle {container}: "
f"{last_error or 'container kill failed'}"
)
_applicator = MacOSContainerEgressApplicator()
didericis marked this conversation as resolved Outdated
Outdated
Review

export the applicator from each of the backends as a singleton instead of exporting a apply_routes_change function wrapper

export the applicator from each of the backends as a singleton instead of exporting a `apply_routes_change` function wrapper
def apply_routes_change(slug: str, content: str) -> tuple[str, str]:
"""Persist `content` to the live routes file and reload egress."""
validate_routes_content(content)
routes_path = _routes_path(slug)
routes_path.parent.mkdir(parents=True, exist_ok=True)
before = routes_path.read_text(encoding="utf-8") if routes_path.exists() else ""
routes_path.write_text(content, encoding="utf-8")
routes_path.chmod(0o600)
_signal_bundle_reload(slug)
return before, content
return _applicator.apply_routes_change(slug, content)
def _routes_path(slug: str) -> Path:
return egress_state_dir(slug) / EGRESS_ROUTES_FILENAME
def _signal_bundle_reload(slug: str) -> None:
container = sidecar_container_name(slug)
result = subprocess.run(
["container", "kill", "--signal", "HUP", container],
capture_output=True, text=True, check=False, env=os.environ,
)
if result.returncode != 0:
last_error = (result.stderr or "").strip() or (result.stdout or "").strip()
warn(
f"egress: routes updated on disk for {slug}, but bundle reload failed: "
f"{last_error or 'container kill failed'}"
)
raise EgressApplyError(
f"could not reload egress bundle {container}: "
f"{last_error or 'container kill failed'}"
)
__all__ = ["apply_routes_change"]
__all__ = ["MacOSContainerEgressApplicator", "EgressApplyError", "apply_routes_change"]