fix: E2E test

.github/workflows/pre-commit.yml

@@ -23,8 +23,11 @@ jobs:
         with:
           python-version: "3.11"
 
-      - name: Install pre-commit
-        run: pip install pre-commit
+      - name: Install pre-commit (and formatters)
+        run: |
+          # Install black/isort explicitly so CI doesn't fail due to missing tools
+          # if a hook is configured to run via system python.
+          pip install pre-commit black==24.1.1 isort==5.13.2
 
       - name: Run all pre-commit hooks
         run: pre-commit run --all-files --show-diff-on-failure

.pre-commit-config.yaml

@@ -42,13 +42,14 @@ repos:
       - id: check-added-large-files
         args: ['--maxkb=500']
 
-  # Python formatting (single-file execution to avoid multiprocessing socket issues)
+  # Python formatting (cross-platform, single-file runner to avoid black multiprocessing issues)
   - repo: local
     hooks:
       - id: black-single
-        name: black (single-file, no multiprocessing)
-        entry: bash -lc 'status=0; for f in "$@"; do python3 -m black --check --diff "$f" || status=$?; done; exit $status' --
-        language: system
+        name: black (single-file)
+        entry: python scripts/precommit_black_single.py
+        language: python
+        additional_dependencies: ['black==24.1.1']
         types: [python]
         # Remove --check --diff to auto-format
 

README.md

@@ -308,8 +308,3 @@ python3 -m unittest discover -s tests -p "test_*.py"
 ## Security
 
 Read `SECURITY.md` before exposing any endpoint beyond localhost. The project is designed to be secure-by-default (deny-by-default auth, SSRF protections, redaction, bounded outputs), but unsafe deployment can still create risk.
-
-## Project planning
-
-- `ROADMAP.md` tracks feature status and priorities.
-- `.planning/` contains detailed plans and implementation records.

api/routes.py

@@ -44,7 +44,11 @@ if web is not None:
             llm_test_handler,
         )
         from ..api.preflight_handler import inventory_handler, preflight_handler
-        from ..api.secrets import secrets_delete_handler, secrets_put_handler
+        from ..api.secrets import (
+            secrets_delete_handler,
+            secrets_put_handler,
+            secrets_status_handler,
+        )
         from ..api.webhook import webhook_handler
         from ..api.webhook_submit import webhook_submit_handler
         from ..api.webhook_validate import webhook_validate_handler
@@ -52,7 +56,15 @@ if web is not None:
         # IMPORTANT: use PACK_VERSION / PACK_START_TIME from config.
         # Do NOT import VERSION or config_path (they do not exist) or route registration will fail.
         from ..config import LOG_FILE, PACK_NAME, PACK_START_TIME, PACK_VERSION
+
+        # CRITICAL: These imports MUST remain present.
+        # If edited out, module-level placeholders stay as None and handlers raise at runtime
+        # (e.g., TypeError: 'NoneType' object is not callable), producing noisy aiohttp tracebacks.
+        from ..services.access_control import require_observability_access
+        from ..services.log_tail import tail_log
+        from ..services.metrics import metrics
         from ..services.rate_limit import check_rate_limit
+        from ..services.redaction import redact_text
 
         # IMPORTANT: services.trace does NOT expose a `trace` symbol.
         # Do not import `trace` here or route registration will fail.
@@ -72,13 +84,21 @@ if web is not None:
             llm_test_handler,
         )
         from api.preflight_handler import inventory_handler, preflight_handler
-        from api.secrets import secrets_delete_handler, secrets_put_handler
+        from api.secrets import (
+            secrets_delete_handler,
+            secrets_put_handler,
+            secrets_status_handler,
+        )
         from api.webhook import webhook_handler
         from api.webhook_submit import webhook_submit_handler
         from api.webhook_validate import webhook_validate_handler
 
         # IMPORTANT: keep PACK_* imports aligned with config.py (VERSION/config_path do not exist).
         from config import LOG_FILE, PACK_NAME, PACK_START_TIME, PACK_VERSION
+        from services.access_control import require_observability_access  # type: ignore
+        from services.log_tail import tail_log  # type: ignore
+        from services.metrics import metrics  # type: ignore
+        from services.rate_limit import check_rate_limit  # type: ignore
         from services.redaction import redact_text  # type: ignore
 
         # IMPORTANT: services.trace does NOT expose a `trace` symbol.
@@ -95,6 +115,29 @@ def check_dependency(module_name: str) -> bool:
         return False
 
 
+def _ensure_observability_deps_ready() -> tuple[bool, str | None]:
+    """
+    Defensive guard against a recurring class of regressions:
+    if the import block above is edited incorrectly, the module-level
+    placeholders stay as None and handlers raise TypeError at runtime.
+    """
+    missing: list[str] = []
+    if not callable(require_observability_access):
+        missing.append("require_observability_access")
+    if not callable(check_rate_limit):
+        missing.append("check_rate_limit")
+    if not callable(tail_log):
+        missing.append("tail_log")
+    if missing:
+        return (
+            False,
+            "Backend not fully initialized (missing route dependencies: "
+            + ", ".join(missing)
+            + ").",
+        )
+    return True, None
+
+
 async def health_handler(request: web.Request) -> web.Response:
     """
     GET /openclaw/health (legacy: /moltbot/health)
@@ -201,6 +244,9 @@ async def logs_tail_handler(request: web.Request) -> web.Response:
     """GET /moltbot/logs/tail - Returns the last N lines of the log file."""
     if web is None:
         raise RuntimeError("aiohttp not available")
+    ok, init_error = _ensure_observability_deps_ready()
+    if not ok:
+        return web.json_response({"ok": False, "error": init_error}, status=500)
     # S14: Access Control
     allowed, error = require_observability_access(request)
     if not allowed:
@@ -301,6 +347,9 @@ async def trace_handler(request: web.Request) -> web.Response:
     """GET /moltbot/trace/{prompt_id} - Returns trace_id and redacted timeline."""
     if web is None:
         raise RuntimeError("aiohttp not available")
+    ok, init_error = _ensure_observability_deps_ready()
+    if not ok:
+        return web.json_response({"ok": False, "error": init_error}, status=500)
     allowed, error = require_observability_access(request)
     if not allowed:
         return web.json_response({"ok": False, "error": error}, status=403)

scripts/precommit_black_single.py

@@ -0,0 +1,35 @@
+from __future__ import annotations
+
+import subprocess
+import sys
+from pathlib import Path
+
+
+def _run_black_on_file(path: str) -> int:
+    # Run black one file at a time to avoid multiprocessing Manager/socket issues
+    # seen in some restricted environments.
+    res = subprocess.run(
+        [sys.executable, "-m", "black", "--check", "--diff", path],
+        stdout=sys.stdout,
+        stderr=sys.stderr,
+        text=False,
+    )
+    return int(res.returncode)
+
+
+def main(argv: list[str]) -> int:
+    # pre-commit passes filenames as argv; we format/check them individually.
+    status = 0
+    for raw in argv[1:]:
+        p = Path(raw)
+        if not p.exists() or p.is_dir():
+            continue
+        rc = _run_black_on_file(str(p))
+        if rc != 0:
+            status = rc
+    return status
+
+
+if __name__ == "__main__":
+    raise SystemExit(main(sys.argv))
+

services/capabilities.py

@@ -26,6 +26,9 @@ def get_capabilities() -> dict:
         "features": {
             "webhook_submit": True,
             "logs_tail": True,
+            # Legacy flag (kept for older frontends/tests).
+            # Do not remove without a migration window + frontend update.
+            "doctor": True,
             "job_monitor": True,
             "callback_delivery": True,
             "presets": True,

services/diagnostics_flags.py

@@ -70,7 +70,9 @@ class ScopedLogger:
     2. Redacts sensitive data before logging.
     """
 
-    def __init__(self, logger: logging.Logger, subsystem: str, manager: DiagnosticsManager):
+    def __init__(
+        self, logger: logging.Logger, subsystem: str, manager: DiagnosticsManager
+    ):
         self._logger = logger
         self._subsystem = subsystem
         self._manager = manager
@@ -88,18 +90,19 @@ class ScopedLogger:
 
         # Prepare message
         prefix = f"[DIAG:{self._subsystem}]"
-        
+
         # Reduct data if present
         if data:
             safe_data = redact_dict_safe(data)
             # Serialize for clarity
             import json
+
             try:
                 # Use default str for non-serializable objects
                 json_part = json.dumps(safe_data, default=str)
             except Exception:
                 json_part = str(safe_data)
-            
+
             self._logger.info(f"{prefix} {msg} | Data: {json_part}", **kwargs)
         else:
             self._logger.info(f"{prefix} {msg}", **kwargs)

tests/TEST_SOP.md

@@ -1,53 +1,52 @@
-# Test SOP
+# Test SOP
 
-This document defines the **mandatory test workflow** for this repo. Follow it **before every push** unless explicitly skipping tests for a scoped reason.
+This document defines the **mandatory test workflow** for this repo. Run it **before every push** (unless you explicitly document why you’re skipping).
 
 ## Prerequisites
-- Python 3.10+
-- Node.js 20+
-- `pre-commit` installed (`pip install pre-commit`)
-- Playwright browsers installed (one-time): `npx playwright install chromium`
+- Python 3.10+ (CI uses 3.10/3.11)
+- Node.js 18+ (CI uses 20)
+- `pre-commit` installed: `python -m pip install pre-commit`
+- Frontend deps installed: `npm install`
 
 ## Required Pre-Push Workflow (Must Run)
 1) Detect Secrets (baseline-based)
-```
+```bash
 pre-commit run detect-secrets --all-files
 ```
 
 2) Run all pre-commit hooks
-```
-pre-commit run --all-files
+```bash
+pre-commit run --all-files --show-diff-on-failure
 ```
 
-3) Frontend E2E (Playwright)
+3) Backend unit tests (recommended; CI enforces)
+```bash
+MOLTBOT_STATE_DIR="$(pwd)/moltbot_state/_local_unit" python -m unittest discover -s tests -p "test_*.py" -v
 ```
-npm install
+
+4) Frontend E2E (Playwright; CI enforces)
+```bash
+# One-time browser install (recommended)
+npx playwright install chromium
+
 npm test
 ```
 
-## Optional (Local Developer Confidence)
-Run only if your environment has Python deps installed.
-```
-python3 -m unittest tests.test_checkpoints -v
-python3 -m unittest tests.test_preflight -v
-python3 -m unittest tests.test_checkpoints_api -v
-python3 -m unittest tests.test_api_model_list -v
-```
+For OS-specific E2E setup (Windows/WSL temp-dir shims), see `tests/E2E_TESTING_SOP.md`.
 
-## CI Equivalence (What the pipeline enforces)
-- Pre-commit hooks (all files)
-- Playwright E2E (`npm test`)
-- Import smoke tests
+## WSL / Restricted Environments
+If `pre-commit` fails due to cache permissions, run with a writable cache directory:
+```bash
+PRE_COMMIT_HOME=/tmp/pre-commit-cache pre-commit run --all-files --show-diff-on-failure
+```
 
 ## Troubleshooting Quick Fixes
 **Detect-secrets fails**
-- Ensure `.secrets.baseline` is up to date.
-- Replace real-looking secrets in docs/examples with `<YOUR_API_KEY>`.
+- Update `.secrets.baseline` (or mark known false positives) and avoid real-looking secrets in docs/tests.
 
-**Playwright fails to install**
-- Ensure `npm` and `node` are on PATH.
-- Reinstall browsers: `npx playwright install chromium`
+**Playwright fails (missing browsers)**
+- Install browsers: `npx playwright install chromium`
 
 **E2E fails with “test harness failed to load”**
-- Check console error in the CI log (module import/exports mismatch).
-- Verify all referenced JS files exist and export the expected names.
+- Check the console error (module import/exports mismatch is the most common cause).
+- Verify all referenced JS modules exist and export expected names.

tests/test_diagnostics_flags.py

@@ -1,6 +1,7 @@
 """
 Unit tests for Diagnostics Flags (R46).
 """
+
 import logging
 import unittest
 from unittest.mock import MagicMock, patch
@@ -19,17 +20,21 @@ class TestDiagnosticsFlags(unittest.TestCase):
 
     def test_glob_matching(self):
         """Test glob pattern matching logic."""
-        with patch.dict("os.environ", {"OPENCLAW_DIAGNOSTICS": "webhook.*, templates.*"}):
+        with patch.dict(
+            "os.environ", {"OPENCLAW_DIAGNOSTICS": "webhook.*, templates.*"}
+        ):
             mgr = DiagnosticsManager()
-            
+
             # Direct matches
             self.assertTrue(mgr.is_enabled("webhook.submit"))
             self.assertTrue(mgr.is_enabled("webhook.validate"))
             self.assertTrue(mgr.is_enabled("templates.render"))
-            
+
             # Non-matches
             self.assertFalse(mgr.is_enabled("llm.client"))
-            self.assertFalse(mgr.is_enabled("webhook")) # "webhook.*" matches "webhook.something", typically not "webhook" unless pattern is "webhook*"
+            self.assertFalse(
+                mgr.is_enabled("webhook")
+            )  # "webhook.*" matches "webhook.something", typically not "webhook" unless pattern is "webhook*"
 
     def test_empty_config(self):
         """Test default safe state."""
@@ -47,18 +52,18 @@ class TestDiagnosticsFlags(unittest.TestCase):
         mgr = DiagnosticsManager()
         # Mock enabled for "test"
         mgr.is_enabled = MagicMock(return_value=True)
-        
+
         mock_logger = MagicMock()
         scoped = ScopedLogger(mock_logger, "test", mgr)
-        
+
         sensitive_data = {"api_key": "sk-123456", "safe": "value"}
         scoped.debug("Test message", data=sensitive_data)
-        
+
         # Verify call args
         mock_logger.info.assert_called_once()
         args, _ = mock_logger.info.call_args
         log_msg = args[0]
-        
+
         self.assertIn("[DIAG:test]", log_msg)
         self.assertIn("***REDACTED***", log_msg)
         self.assertNotIn("sk-123456", log_msg)
@@ -68,12 +73,13 @@ class TestDiagnosticsFlags(unittest.TestCase):
         """Test that disabled logger does nothing."""
         mgr = DiagnosticsManager()
         mgr.is_enabled = MagicMock(return_value=False)
-        
+
         mock_logger = MagicMock()
         scoped = ScopedLogger(mock_logger, "test", mgr)
-        
+
         scoped.debug("Should not log")
         mock_logger.info.assert_not_called()
 
+
 if __name__ == "__main__":
     unittest.main()