refactor: harden optional dependency import parity

This commit is contained in:
rookiestar28
2026-03-27 16:24:13 +08:00
parent b46dbbebae
commit 72c74c153f
18 changed files with 253 additions and 31 deletions
+5 -2
View File
@@ -3,13 +3,14 @@ Approval API Endpoints (S7/F12).
REST endpoints for managing approval requests.
"""
from __future__ import annotations
import logging
from typing import Callable, Optional
from aiohttp import web
try:
from ..services.access_control import resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.approvals.models import ApprovalStatus
from ..services.approvals.service import get_approval_service
from ..services.audit import emit_audit_event
@@ -19,6 +20,7 @@ try:
except ImportError:
# Fallback for ComfyUI's non-package loader or ad-hoc imports.
from services.access_control import resolve_token_info # type: ignore
from services.aiohttp_compat import import_aiohttp_web
from services.approvals.models import ApprovalStatus
from services.approvals.service import get_approval_service
from services.audit import emit_audit_event # type: ignore
@@ -33,6 +35,7 @@ except ImportError:
from services.webhook_auth import AuthError
logger = logging.getLogger("ComfyUI-OpenClaw.api.approvals")
web = import_aiohttp_web()
class ApprovalHandlers:
+5 -2
View File
@@ -1,13 +1,14 @@
from __future__ import annotations
import asyncio
import contextlib
import json
import logging
from typing import Any, Dict, Optional
from aiohttp import web
try:
from ..services.access_control import require_admin_token
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.async_utils import run_in_thread
from ..services.automation_composer import AutomationComposerService
from ..services.planner import PlannerService
@@ -22,6 +23,7 @@ try:
except ImportError:
# Fallback for ComfyUI's non-package loader or ad-hoc imports.
from services.access_control import require_admin_token
from services.aiohttp_compat import import_aiohttp_web
from services.async_utils import run_in_thread
from services.automation_composer import AutomationComposerService
from services.planner import PlannerService
@@ -51,6 +53,7 @@ else:
)
logger = logging.getLogger("ComfyUI-OpenClaw.api.assist")
web = import_aiohttp_web()
# Payload size limits (character count for strings, base64 length for images)
MAX_REQUIREMENTS_LEN = 8000
+5 -1
View File
@@ -3,12 +3,14 @@ Capabilities API Handler (R19).
GET /openclaw/capabilities (legacy: /moltbot/capabilities)
"""
from aiohttp import web
from __future__ import annotations
# Import discipline
if __package__ and "." in __package__:
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.capabilities import get_capabilities
else:
from services.aiohttp_compat import import_aiohttp_web
from services.capabilities import get_capabilities
@@ -27,6 +29,8 @@ else:
endpoint_metadata,
)
web = import_aiohttp_web()
@endpoint_metadata(
auth=AuthTier.PUBLIC,
+3 -2
View File
@@ -7,10 +7,9 @@ from __future__ import annotations
import logging
from typing import Any, Dict, Optional
from aiohttp import web
try:
from ..services.access_control import require_admin_token, resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -25,6 +24,7 @@ except ImportError: # pragma: no cover
require_admin_token,
resolve_token_info,
)
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.endpoint_manifest import ( # type: ignore
AuthTier,
RiskTier,
@@ -39,6 +39,7 @@ except ImportError: # pragma: no cover
)
logger = logging.getLogger("ComfyUI-OpenClaw.api.model_manager")
web = import_aiohttp_web()
def _json(data: Dict[str, Any], status: int = 200) -> web.Response:
+3 -2
View File
@@ -8,10 +8,9 @@ import os
import time
from typing import Optional
from aiohttp import web
try:
from ..services.access_control import require_admin_token, resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -23,6 +22,7 @@ try:
except ImportError:
# Fallback for ComfyUI's non-package loader or ad-hoc imports.
from services.access_control import require_admin_token, resolve_token_info
from services.aiohttp_compat import import_aiohttp_web
from services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -33,6 +33,7 @@ except ImportError:
from services.tenant_context import TenantBoundaryError, request_tenant_scope
logger = logging.getLogger("ComfyUI-OpenClaw.api.presets")
web = import_aiohttp_web()
class PresetHandlers:
+4 -2
View File
@@ -7,9 +7,8 @@ from __future__ import annotations
from pathlib import Path
from aiohttp import web
if __package__ and "." in __package__:
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -17,6 +16,7 @@ if __package__ and "." in __package__:
endpoint_metadata,
)
else: # pragma: no cover (test-only import mode)
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.endpoint_manifest import ( # type: ignore
AuthTier,
RiskTier,
@@ -24,6 +24,8 @@ else: # pragma: no cover (test-only import mode)
endpoint_metadata,
)
web = import_aiohttp_web()
# CRITICAL: resolve HTML path relative to this module's package root.
# Do not switch to cwd-based resolution; ComfyUI may launch from arbitrary directories.
+3 -2
View File
@@ -13,10 +13,9 @@ import logging
import time
from typing import Any, Dict
from aiohttp import web
try:
from ..services.access_control import require_admin_token, resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -39,6 +38,7 @@ except ImportError:
require_admin_token,
resolve_token_info,
)
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.endpoint_manifest import ( # type: ignore
AuthTier,
RiskTier,
@@ -61,6 +61,7 @@ except ImportError:
)
logger = logging.getLogger("ComfyUI-OpenClaw.api.rewrite_recipes")
web = import_aiohttp_web()
def _json(data: Dict[str, Any], status: int = 200) -> web.Response:
+5 -2
View File
@@ -3,11 +3,11 @@ Scheduler CRUD API (R4).
REST endpoints for managing persistent schedules.
"""
from __future__ import annotations
import logging
from typing import Optional
from aiohttp import web
# Import discipline:
# - ComfyUI runtime: package-relative imports only (prevents collisions with other custom nodes).
# - Unit tests: allow top-level fallbacks.
@@ -15,17 +15,20 @@ from aiohttp import web
# IMPORTANT: Avoid a broad `try/except ImportError` here. Falling back to `services.*` in ComfyUI
# can silently import another pack's module and break auth/approval semantics.
if __package__ and "." in __package__:
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.scheduler.models import Schedule, TriggerType
from ..services.scheduler.storage import get_schedule_store
from ..services.templates import is_template_allowed
from ..services.webhook_auth import AuthError
else: # pragma: no cover (test-only import mode)
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.scheduler.models import Schedule, TriggerType # type: ignore
from services.scheduler.storage import get_schedule_store # type: ignore
from services.templates import is_template_allowed # type: ignore
from services.webhook_auth import AuthError # type: ignore
logger = logging.getLogger("ComfyUI-OpenClaw.api.schedules")
web = import_aiohttp_web()
def _get_scheduler_runner():
+6 -2
View File
@@ -15,18 +15,19 @@ Endpoints:
- DELETE /openclaw/secrets/{provider}: Clear provider secret
"""
from __future__ import annotations
import logging
import os
from typing import Optional
from aiohttp import web
# Import discipline:
# - ComfyUI runtime: package-relative imports only.
# - Unit tests: allow top-level fallbacks.
if __package__ and "." in __package__:
from ..models.schemas import MAX_BODY_SIZE
from ..services.access_control import require_admin_token, resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.audit import emit_audit_event
from ..services.csrf_protection import require_same_origin_if_no_token
from ..services.metrics import metrics
@@ -38,6 +39,7 @@ else: # pragma: no cover (test-only import mode)
from models.schemas import MAX_BODY_SIZE # type: ignore
from services.access_control import require_admin_token # type: ignore
from services.access_control import resolve_token_info # type: ignore
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.audit import emit_audit_event # type: ignore
from services.csrf_protection import require_same_origin_if_no_token # type: ignore
from services.metrics import metrics # type: ignore
@@ -50,6 +52,8 @@ else: # pragma: no cover (test-only import mode)
from services.runtime_config import is_loopback_client
from services.secret_store import get_secret_store # type: ignore
web = import_aiohttp_web()
# R98: Endpoint Metadata
if __package__ and "." in __package__:
from ..services.endpoint_manifest import (
+5 -2
View File
@@ -3,18 +3,20 @@ S12: API Handlers for External Tools.
Protected by Admin Token and Feature Flag.
"""
from __future__ import annotations
import json
import logging
from aiohttp import web
try:
from ..services.access_control import require_admin_token, resolve_token_info
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.audit import emit_audit_event
from ..services.tool_runner import get_tool_runner, is_tools_enabled
except ImportError:
from services.access_control import require_admin_token # type: ignore
from services.access_control import resolve_token_info # type: ignore
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.audit import emit_audit_event # type: ignore
from services.tool_runner import get_tool_runner, is_tools_enabled
@@ -35,6 +37,7 @@ else:
)
logger = logging.getLogger("ComfyUI-OpenClaw.api.tools")
web = import_aiohttp_web()
@endpoint_metadata(
+5 -2
View File
@@ -4,14 +4,14 @@ Endpoint for firing workflow triggers from external systems.
With S7 approval gate support.
"""
from __future__ import annotations
import hashlib
import json
import logging
import os
from typing import Optional
from aiohttp import web
# Import discipline:
# - ComfyUI runtime: this pack is loaded as a package; MUST use package-relative imports to avoid
# collisions with other custom nodes or other top-level modules named `services`.
@@ -23,6 +23,7 @@ from aiohttp import web
# can silently import the WRONG module (another custom node or ComfyUI-adjacent package), causing
# template allowlists to appear "missing" even when `data/templates/manifest.json` is correct.
if __package__ and "." in __package__:
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.endpoint_manifest import (
AuthTier,
RiskTier,
@@ -34,6 +35,7 @@ if __package__ and "." in __package__:
from ..services.trace import generate_trace_id
from ..services.webhook_auth import AuthError
else: # pragma: no cover (test-only import mode)
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.endpoint_manifest import ( # type: ignore
AuthTier,
RiskTier,
@@ -46,6 +48,7 @@ else: # pragma: no cover (test-only import mode)
from services.webhook_auth import AuthError # type: ignore
logger = logging.getLogger("ComfyUI-OpenClaw.api.triggers")
web = import_aiohttp_web()
# Default: require approval for external triggers (secure-by-default)
REQUIRE_APPROVAL_DEFAULT = (
+5 -2
View File
@@ -9,11 +9,11 @@ POST /moltbot/webhook
- Returns normalized internal request
"""
from __future__ import annotations
import json
import logging
from aiohttp import web
try:
from .errors import APIError, ErrorCode, create_error_response
except ImportError:
@@ -22,12 +22,14 @@ except ImportError:
try:
from ..models.schemas import MAX_BODY_SIZE, WebhookJobRequest
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.metrics import metrics
from ..services.rate_limit import build_rate_limit_response, check_rate_limit
from ..services.trace import get_effective_trace_id
from ..services.webhook_auth import get_auth_summary, require_auth
except ImportError:
from models.schemas import MAX_BODY_SIZE, WebhookJobRequest
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.metrics import metrics
from services.rate_limit import build_rate_limit_response, check_rate_limit
from services.trace import get_effective_trace_id
@@ -52,6 +54,7 @@ except ImportError:
# R46: Scoped logger for safe-by-default redaction
logger = diagnostics.get_logger("ComfyUI-OpenClaw.api.webhook", "webhook")
web = import_aiohttp_web()
@endpoint_metadata(
+3 -2
View File
@@ -6,8 +6,6 @@ Connects S2 (Auth) -> R8 (Normalization) -> R3 (Idempotency) -> F5 (Execution).
import json
import logging
from aiohttp import web
# Import discipline:
# - ComfyUI runtime: package-relative imports only (prevents collisions with other custom nodes).
# - Unit tests: allow top-level fallbacks.
@@ -17,6 +15,7 @@ from aiohttp import web
# import another pack's top-level `services` module and break allowlists/auth in surprising ways.
if __package__ and "." in __package__:
from ..models.schemas import MAX_BODY_SIZE, WebhookJobRequest
from ..services.aiohttp_compat import import_aiohttp_web
from ..services.callback_delivery import start_callback_watch
from ..services.execution_budgets import BudgetExceededError
from ..services.idempotency_store import IdempotencyStore
@@ -35,6 +34,7 @@ if __package__ and "." in __package__:
)
else: # pragma: no cover (test-only import mode)
from models.schemas import MAX_BODY_SIZE, WebhookJobRequest
from services.aiohttp_compat import import_aiohttp_web # type: ignore
from services.callback_delivery import start_callback_watch # type: ignore
from services.execution_budgets import BudgetExceededError # type: ignore
from services.idempotency_store import IdempotencyStore # type: ignore
@@ -72,6 +72,7 @@ else:
)
logger = logging.getLogger("ComfyUI-OpenClaw.api.webhook_submit")
web = import_aiohttp_web()
def safe_error_response(status: int, error: str, detail: str = "") -> web.Response:
+58
View File
@@ -0,0 +1,58 @@
"""
R145 aiohttp compatibility helpers.
Keeps route/service modules importable in minimal environments where aiohttp is
not installed, while still failing explicitly when request/response helpers are
actually invoked.
"""
from __future__ import annotations
from typing import Any
def _missing_aiohttp(*_args: Any, **_kwargs: Any) -> Any:
raise RuntimeError("aiohttp not available")
class _UnavailableResponse:
def __init__(self, *_args: Any, **_kwargs: Any) -> None:
_missing_aiohttp()
class _UnavailableStreamResponse:
def __init__(self, *_args: Any, **_kwargs: Any) -> None:
_missing_aiohttp()
class _MissingAiohttpWeb:
__openclaw_aiohttp_available__ = False
Request = object
Application = object
Response = _UnavailableResponse
StreamResponse = _UnavailableStreamResponse
def json_response(self, *_args: Any, **_kwargs: Any) -> Any:
_missing_aiohttp()
def __getattr__(self, _name: str) -> Any:
return _missing_aiohttp
MISSING_AIOHTTP_WEB = _MissingAiohttpWeb()
def import_aiohttp_web() -> Any:
"""
Return `aiohttp.web` when available, otherwise a fail-fast shim.
CRITICAL: only swallow ModuleNotFoundError for aiohttp itself. Broader
ImportError handling can hide unrelated packaged-context regressions.
"""
try:
from aiohttp import web # type: ignore
except ModuleNotFoundError as exc: # pragma: no cover - minimal env path
if exc.name != "aiohttp":
raise
return MISSING_AIOHTTP_WEB
return web
+8 -1
View File
@@ -10,10 +10,17 @@ Security policy:
- profile=public/hardened: fail-CLOSED on errors (security-first)
"""
from __future__ import annotations
import logging
import os
from aiohttp import web
try:
from .aiohttp_compat import import_aiohttp_web
except ImportError:
from aiohttp_compat import import_aiohttp_web # type: ignore
web = import_aiohttp_web()
logger = logging.getLogger(__name__)
+3 -3
View File
@@ -3,7 +3,7 @@ import unittest
from pathlib import Path
from unittest.mock import MagicMock, patch
from api.remote_admin import remote_admin_page_handler
import api.remote_admin as remote_admin
class TestF61RemoteAdminConsole(unittest.IsolatedAsyncioTestCase):
@@ -16,7 +16,7 @@ class TestF61RemoteAdminConsole(unittest.IsolatedAsyncioTestCase):
with patch(
"api.remote_admin._admin_console_html_path", return_value=html_path
):
resp = await remote_admin_page_handler(request)
resp = await remote_admin.remote_admin_page_handler(request)
self.assertEqual(resp.status, 200)
self.assertIn("text/html", resp.content_type)
@@ -28,7 +28,7 @@ class TestF61RemoteAdminConsole(unittest.IsolatedAsyncioTestCase):
"api.remote_admin._admin_console_html_path",
return_value=Path("__missing_admin_console__.html"),
):
resp = await remote_admin_page_handler(request)
resp = await remote_admin.remote_admin_page_handler(request)
self.assertEqual(resp.status, 500)
self.assertIn("remote_admin_console_not_found", resp.text)
+1 -2
View File
@@ -14,8 +14,7 @@ class TestR144ContractMatrix(unittest.TestCase):
def test_documented_routes_cover_r144_fixture_paths(self):
documented = {
(route.method, route.path)
for route in parse_api_contract_markdown()
(route.method, route.path) for route in parse_api_contract_markdown()
}
bundle = get_serializable_contract_bundle()
missing = []
+126
View File
@@ -0,0 +1,126 @@
"""
R145 optional-dependency lazy-import parity regressions.
"""
from __future__ import annotations
import asyncio
import importlib
import os
import sys
import unittest
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import patch
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
MODULES_UNDER_TEST = [
"api.approvals",
"api.assist",
"api.capabilities",
"api.model_manager",
"api.presets",
"api.remote_admin",
"api.rewrite_recipes",
"api.schedules",
"api.secrets",
"api.tools",
"api.triggers",
"api.webhook",
"api.webhook_submit",
"services.surface_guard",
]
def _purge_module(name: str) -> None:
sys.modules.pop(name, None)
def _capture_parent_binding(name: str):
parent_name, _, attr_name = name.rpartition(".")
if not parent_name:
return None, attr_name, False, None
parent = sys.modules.get(parent_name)
if parent is None:
parent = importlib.import_module(parent_name)
had_attr = hasattr(parent, attr_name)
return parent, attr_name, had_attr, getattr(parent, attr_name, None)
def _restore_parent_binding(parent, attr_name: str, had_attr: bool, value) -> None:
if parent is None:
return
if had_attr:
setattr(parent, attr_name, value)
else:
parent.__dict__.pop(attr_name, None)
def _import_without_aiohttp(module_name: str):
original_module = sys.modules.get(module_name)
original_compat = sys.modules.get("services.aiohttp_compat")
original_module_parent = _capture_parent_binding(module_name)
original_compat_parent = _capture_parent_binding("services.aiohttp_compat")
_purge_module(module_name)
_purge_module("services.aiohttp_compat")
try:
with patch.dict(sys.modules, {"aiohttp": None}):
return importlib.import_module(module_name)
finally:
if original_module is not None:
sys.modules[module_name] = original_module
else:
_purge_module(module_name)
_restore_parent_binding(*original_module_parent)
if original_compat is not None:
sys.modules["services.aiohttp_compat"] = original_compat
else:
_purge_module("services.aiohttp_compat")
_restore_parent_binding(*original_compat_parent)
class TestR145OptionalDependencyParity(unittest.TestCase):
def test_route_modules_import_without_aiohttp(self):
for module_name in MODULES_UNDER_TEST:
with self.subTest(module=module_name):
module = _import_without_aiohttp(module_name)
self.assertTrue(hasattr(module, "web"))
self.assertFalse(
getattr(module.web, "__openclaw_aiohttp_available__", True)
)
def test_json_helpers_fail_explicitly_without_aiohttp(self):
model_manager = _import_without_aiohttp("api.model_manager")
webhook_submit = _import_without_aiohttp("api.webhook_submit")
with self.assertRaisesRegex(RuntimeError, "aiohttp not available"):
model_manager._json({"ok": True})
with self.assertRaisesRegex(RuntimeError, "aiohttp not available"):
webhook_submit.safe_error_response(400, "bad_request")
def test_async_handlers_fail_explicitly_without_aiohttp(self):
capabilities = _import_without_aiohttp("api.capabilities")
remote_admin = _import_without_aiohttp("api.remote_admin")
with self.assertRaisesRegex(RuntimeError, "aiohttp not available"):
asyncio.run(capabilities.capabilities_handler(SimpleNamespace()))
with self.assertRaisesRegex(RuntimeError, "aiohttp not available"):
asyncio.run(remote_admin.remote_admin_page_handler(SimpleNamespace()))
def test_surface_guard_stays_importable_in_local_mode(self):
surface_guard = _import_without_aiohttp("services.surface_guard")
with patch.dict(os.environ, {"OPENCLAW_DEPLOYMENT_PROFILE": "local"}, clear=False):
self.assertIsNone(surface_guard.check_surface("nonexistent-surface"))
def test_remote_admin_fixture_exists(self):
remote_admin = importlib.import_module("api.remote_admin")
self.assertTrue(Path(remote_admin._admin_console_html_path()).exists())
if __name__ == "__main__":
unittest.main()