From c1365130d315aa09a5bcfe3e84e8c8771d0a0e5c Mon Sep 17 00:00:00 2001 From: rookiestar28 Date: Thu, 19 Mar 2026 18:47:49 +0800 Subject: [PATCH] refactor: harden route bootstrap contract --- services/route_bootstrap.py | 118 +++++++++---------------- services/route_bootstrap_contract.py | 95 ++++++++++++++++++++ tests/test_route_bootstrap_contract.py | 58 ++++++++++++ 3 files changed, 194 insertions(+), 77 deletions(-) create mode 100644 services/route_bootstrap_contract.py create mode 100644 tests/test_route_bootstrap_contract.py diff --git a/services/route_bootstrap.py b/services/route_bootstrap.py index 5ad8239..88c08fa 100644 --- a/services/route_bootstrap.py +++ b/services/route_bootstrap.py @@ -82,48 +82,19 @@ def _initialize_registries_and_security_gate() -> None: def _do_full_registration(server) -> None: """Register all OpenClaw routes including bridge/scheduler bindings.""" from .access_control import require_admin_token - from .import_fallback import import_attrs_dual from .plugins.async_bridge import run_async_in_sync_context from .queue_submit import submit_prompt + from .route_bootstrap_contract import load_route_bootstrap_contract from .scheduler.runner import get_scheduler_runner, start_scheduler from .templates import get_template_service - (register_approval_routes,) = import_attrs_dual( - __package__, - "..api.approvals", - "api.approvals", - ("register_approval_routes",), - ) - (BridgeHandlers,) = import_attrs_dual( - __package__, - "..api.bridge", - "api.bridge", - ("BridgeHandlers",), - ) - (register_preset_routes,) = import_attrs_dual( - __package__, - "..api.presets", - "api.presets", - ("register_preset_routes",), - ) - (register_routes,) = import_attrs_dual( - __package__, - "..api.routes", - "api.routes", - ("register_routes",), - ) - (register_schedule_routes,) = import_attrs_dual( - __package__, - "..api.schedules", - "api.schedules", - ("register_schedule_routes",), - ) - (register_trigger_routes,) = import_attrs_dual( - __package__, - "..api.triggers", - "api.triggers", - ("register_trigger_routes",), - ) + contract = load_route_bootstrap_contract(__package__) + register_approval_routes = contract["register_approval_routes"] + BridgeHandlers = contract["BridgeHandlers"] + register_preset_routes = contract["register_preset_routes"] + register_routes = contract["register_routes"] + register_schedule_routes = contract["register_schedule_routes"] + register_trigger_routes = contract["register_trigger_routes"] register_routes(server) register_preset_routes(server.app) @@ -150,46 +121,7 @@ def _do_full_registration(server) -> None: return run_async_in_sync_context(_do_submit()) bridge_handlers = BridgeHandlers(submit_service=QueueSubmitService()) - if hasattr(server.app.router, "add_post"): - server.app.router.add_post( - "/moltbot/bridge/submit", bridge_handlers.submit_handler - ) - server.app.router.add_post( - "/moltbot/bridge/deliver", bridge_handlers.deliver_handler - ) - server.app.router.add_get( - "/moltbot/bridge/health", bridge_handlers.health_handler - ) - server.app.router.add_post( - "/openclaw/bridge/submit", bridge_handlers.submit_handler - ) - server.app.router.add_post( - "/openclaw/bridge/deliver", bridge_handlers.deliver_handler - ) - server.app.router.add_get( - "/openclaw/bridge/health", bridge_handlers.health_handler - ) - try: - server.app.router.add_post( - "/api/moltbot/bridge/submit", bridge_handlers.submit_handler - ) - server.app.router.add_post( - "/api/moltbot/bridge/deliver", bridge_handlers.deliver_handler - ) - server.app.router.add_get( - "/api/moltbot/bridge/health", bridge_handlers.health_handler - ) - server.app.router.add_post( - "/api/openclaw/bridge/submit", bridge_handlers.submit_handler - ) - server.app.router.add_post( - "/api/openclaw/bridge/deliver", bridge_handlers.deliver_handler - ) - server.app.router.add_get( - "/api/openclaw/bridge/health", bridge_handlers.health_handler - ) - except RuntimeError: - pass + _register_bridge_routes(server.app.router, bridge_handlers) async def unified_submit_fn( template_id, @@ -244,6 +176,38 @@ def _do_full_registration(server) -> None: ) +_BRIDGE_ROUTE_SPECS = ( + ("add_post", "/moltbot/bridge/submit", "submit_handler"), + ("add_post", "/moltbot/bridge/deliver", "deliver_handler"), + ("add_get", "/moltbot/bridge/health", "health_handler"), + ("add_post", "/openclaw/bridge/submit", "submit_handler"), + ("add_post", "/openclaw/bridge/deliver", "deliver_handler"), + ("add_get", "/openclaw/bridge/health", "health_handler"), + ("add_post", "/api/moltbot/bridge/submit", "submit_handler"), + ("add_post", "/api/moltbot/bridge/deliver", "deliver_handler"), + ("add_get", "/api/moltbot/bridge/health", "health_handler"), + ("add_post", "/api/openclaw/bridge/submit", "submit_handler"), + ("add_post", "/api/openclaw/bridge/deliver", "deliver_handler"), + ("add_get", "/api/openclaw/bridge/health", "health_handler"), +) + + +def _register_bridge_routes(router, bridge_handlers) -> None: + # IMPORTANT: keep bridge route registration table-driven. + # Missing one alias path here silently breaks one control-plane surface while + # leaving the rest apparently healthy, which is hard to diagnose during startup. + for method_name, path, handler_name in _BRIDGE_ROUTE_SPECS: + registrar = getattr(router, method_name, None) + if registrar is None: + continue + try: + registrar(path, getattr(bridge_handlers, handler_name)) + except RuntimeError: + if path.startswith("/api/"): + continue + raise + + def _start_registration_retry_loop() -> None: """R25: Retry route registration while PromptServer is warming up.""" diff --git a/services/route_bootstrap_contract.py b/services/route_bootstrap_contract.py new file mode 100644 index 0000000..a02f8bc --- /dev/null +++ b/services/route_bootstrap_contract.py @@ -0,0 +1,95 @@ +""" +R147 bootstrap contract helpers. + +Keeps route bootstrap imports declarative and validates symbol shape before +runtime registration starts mutating the ComfyUI server/router state. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +from .import_fallback import import_attrs_dual + + +@dataclass(frozen=True) +class BootstrapSymbolSpec: + key: str + relative_module: str + absolute_module: str + attr: str + kind: str + + +ROUTE_BOOTSTRAP_SPECS = ( + BootstrapSymbolSpec( + key="register_approval_routes", + relative_module="..api.approvals", + absolute_module="api.approvals", + attr="register_approval_routes", + kind="callable", + ), + BootstrapSymbolSpec( + key="BridgeHandlers", + relative_module="..api.bridge", + absolute_module="api.bridge", + attr="BridgeHandlers", + kind="class", + ), + BootstrapSymbolSpec( + key="register_preset_routes", + relative_module="..api.presets", + absolute_module="api.presets", + attr="register_preset_routes", + kind="callable", + ), + BootstrapSymbolSpec( + key="register_routes", + relative_module="..api.routes", + absolute_module="api.routes", + attr="register_routes", + kind="callable", + ), + BootstrapSymbolSpec( + key="register_schedule_routes", + relative_module="..api.schedules", + absolute_module="api.schedules", + attr="register_schedule_routes", + kind="callable", + ), + BootstrapSymbolSpec( + key="register_trigger_routes", + relative_module="..api.triggers", + absolute_module="api.triggers", + attr="register_trigger_routes", + kind="callable", + ), +) + + +def _validate_symbol(spec: BootstrapSymbolSpec, value: Any) -> None: + if spec.kind == "callable" and not callable(value): + raise RuntimeError( + f"Bootstrap contract violation: {spec.key} from {spec.absolute_module} " + f"must be callable, got {type(value).__name__}" + ) + if spec.kind == "class" and not isinstance(value, type): + raise RuntimeError( + f"Bootstrap contract violation: {spec.key} from {spec.absolute_module} " + f"must be a class, got {type(value).__name__}" + ) + + +def load_route_bootstrap_contract(package_name: str | None) -> dict[str, Any]: + contract: dict[str, Any] = {} + for spec in ROUTE_BOOTSTRAP_SPECS: + (value,) = import_attrs_dual( + package_name, + spec.relative_module, + spec.absolute_module, + (spec.attr,), + ) + _validate_symbol(spec, value) + contract[spec.key] = value + return contract diff --git a/tests/test_route_bootstrap_contract.py b/tests/test_route_bootstrap_contract.py new file mode 100644 index 0000000..64ac79b --- /dev/null +++ b/tests/test_route_bootstrap_contract.py @@ -0,0 +1,58 @@ +import unittest +from unittest.mock import patch + +from services.route_bootstrap import _register_bridge_routes +from services.route_bootstrap_contract import load_route_bootstrap_contract + + +class DummyBridgeHandlers: + def submit_handler(self, request=None): + return request + + def deliver_handler(self, request=None): + return request + + def health_handler(self, request=None): + return request + + +class DummyRouter: + def __init__(self): + self.calls = [] + + def add_post(self, path, handler): + self.calls.append(("POST", path, handler)) + + def add_get(self, path, handler): + self.calls.append(("GET", path, handler)) + + +class RouteBootstrapContractTests(unittest.TestCase): + def test_contract_loader_rejects_non_callable_registrar(self): + def fake_import(_pkg, _rel, _abs, attrs): + if attrs == ("BridgeHandlers",): + return (DummyBridgeHandlers,) + return (object(),) + + with patch( + "services.route_bootstrap_contract.import_attrs_dual", + side_effect=fake_import, + ): + with self.assertRaises(RuntimeError) as ctx: + load_route_bootstrap_contract("services") + + self.assertIn("must be callable", str(ctx.exception)) + + def test_bridge_route_table_registers_all_aliases(self): + router = DummyRouter() + + _register_bridge_routes(router, DummyBridgeHandlers()) + + paths = [path for _method, path, _handler in router.calls] + self.assertEqual(len(paths), 12) + self.assertIn("/openclaw/bridge/submit", paths) + self.assertIn("/api/openclaw/bridge/health", paths) + + +if __name__ == "__main__": + unittest.main()