fix(windows): installer encoding + Ollama readiness loop (#523)

Two bugs found during end-to-end testing on a clean Windows 11 24H2
Azure VM (closes #522). Both are dodged by the canonical `irm | iex`
one-liner but hit by the documented `-OutFile` fallback and any
non-interactive run.

1. Encoding. install.ps1 was UTF-8 without a BOM and contained em-dashes
   plus a box-drawing banner. Windows PowerShell 5.1 decodes BOM-less
   files with the legacy ANSI/OEM code page, mis-decoding the multi-byte
   sequences and desyncing the parser into cascading here-string parse
   errors. Converted the file to pure ASCII (em-dashes -> hyphens, banner
   -> ASCII art) so it parses no matter how it's read.

2. Ollama readiness loop. With $ErrorActionPreference='Stop', the probe
   `& $ollamaExe list 2>&1 | Out-Null` turned the daemon-not-up stderr
   into a terminating NativeCommandError, aborting the install on the
   first iteration and making the loop's own Start-Process serve retry +
   Write-Warn2 fallback dead code. Wrapped the probe in try/catch so it
   falls through to the self-start path as intended.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

configs/openjarvis/config.toml

@@ -106,6 +106,11 @@ enabled = true                            # Record traces for analysis
 db_path = "~/.openjarvis/traces.db"
 
 [server]
-host = "0.0.0.0"
+# Bind to loopback by default so the API is not exposed to the local network.
+# To serve other devices on your LAN, set host = "0.0.0.0" AND set an API key
+# (OPENJARVIS_API_KEY / `jarvis auth generate-key`) — startup refuses a
+# non-loopback bind without a key. The "server" security profile also flips
+# this to 0.0.0.0 intentionally.
+host = "127.0.0.1"
 port = 8000
 agent = "native_openhands"

deploy/windows/install.ps1

@@ -5,12 +5,12 @@
 .DESCRIPTION
     Phase-1 of the native-Windows-support RFC (#298). Mirrors the
     behavior of scripts/install/install.sh (the curl-pipe-bash installer
-    for Linux/WSL2/macOS) but for native Windows PowerShell — no WSL,
+    for Linux/WSL2/macOS) but for native Windows PowerShell - no WSL,
     no Docker, no MSYS2.
 
     Steps:
       1. Refuse non-Windows / Windows < 10.
-      2. Check Python 3.10 — 3.13 on PATH (3.14 has no numpy wheels yet,
+      2. Check Python 3.10 - 3.13 on PATH (3.14 has no numpy wheels yet,
          see #432).
       3. Check git on PATH.
       4. Install uv (https://astral.sh/uv) if absent.
@@ -65,7 +65,7 @@ if (-not $Service     -and $env:OPENJARVIS_SERVICE)      { $Service     = $true
 if (-not $Force       -and $env:OPENJARVIS_FORCE)        { $Force       = $true }
 
 # ---------------------------------------------------------------------------
-# Output helpers — coloured but plain enough for Constrained Language Mode.
+# Output helpers - coloured but plain enough for Constrained Language Mode.
 # ---------------------------------------------------------------------------
 
 function Write-Info  ($msg) { Write-Host "[info]  $msg" -ForegroundColor Cyan }
@@ -77,13 +77,13 @@ function Write-Fail  ($msg) {
 }
 
 # ---------------------------------------------------------------------------
-# Shared helpers — winget bootstrap + PATH refresh
+# Shared helpers - winget bootstrap + PATH refresh
 # ---------------------------------------------------------------------------
 
 # Pull the latest Machine + User PATH from the registry into the current
 # PowerShell session. Tools installed by `winget install` (Python, git,
 # Ollama, etc.) update the User PATH, but the running process inherits
-# the parent shell's environment — so without this refresh the just-
+# the parent shell's environment - so without this refresh the just-
 # installed tool stays invisible to subsequent `Get-Command` calls.
 #
 # CRITICAL: registry PATH entries can be REG_EXPAND_SZ (with literal
@@ -157,7 +157,7 @@ function Get-PythonCommand {
 Write-Info "Checking Python (3.10 - 3.13)..."
 $pythonExe = Get-PythonCommand
 if (-not $pythonExe) {
-    Write-Info "Python not on PATH — attempting auto-install via winget..."
+    Write-Info "Python not on PATH - attempting auto-install via winget..."
     $pythonExe = Install-WithWinget -WingetId 'Python.Python.3.13' -CommandName 'python'
     if (-not $pythonExe) {
         Write-Fail @"
@@ -196,7 +196,7 @@ Write-Ok "Python $pyMajor.$pyMinor ($pythonExe)"
 Write-Info "Checking git..."
 $gitExe = (Get-Command git -ErrorAction SilentlyContinue).Source
 if (-not $gitExe) {
-    Write-Info "git not on PATH — attempting auto-install via winget..."
+    Write-Info "git not on PATH - attempting auto-install via winget..."
     $gitExe = Install-WithWinget -WingetId 'Git.Git' -CommandName 'git'
     if (-not $gitExe) {
         Write-Fail @"
@@ -227,7 +227,7 @@ if (-not $uvExe) {
     }
     # The astral installer puts uv at %USERPROFILE%\.local\bin\uv.exe and
     # adds that dir to the User PATH. The current process's PATH isn't
-    # refreshed automatically — prepend the install dir so the rest of
+    # refreshed automatically - prepend the install dir so the rest of
     # this script picks it up.
     $uvDir = Join-Path $env:USERPROFILE '.local\bin'
     if (Test-Path (Join-Path $uvDir 'uv.exe')) {
@@ -295,13 +295,13 @@ try {
 Write-Ok "Dependencies installed"
 
 # ---------------------------------------------------------------------------
-# 7. Ollama — install + start + wait for daemon
+# 7. Ollama - install + start + wait for daemon
 # ---------------------------------------------------------------------------
 
 Write-Info "Checking Ollama..."
 $ollamaExe = (Get-Command ollama -ErrorAction SilentlyContinue).Source
 if (-not $ollamaExe) {
-    Write-Info "  Ollama not on PATH — downloading the official installer (~150 MB)..."
+    Write-Info "  Ollama not on PATH - downloading the official installer (~150 MB)..."
     $ollamaSetup = Join-Path $env:TEMP 'OllamaSetup.exe'
     # SilentlyContinue is load-bearing in PS 5.1: the default progress
     # bar renderer slows Invoke-WebRequest down 30x on large downloads
@@ -340,13 +340,18 @@ Write-Ok "Ollama ($ollamaExe)"
 Write-Info "Waiting for Ollama daemon..."
 $ollamaReady = $false
 for ($i = 0; $i -lt 60; $i++) {
-    & $ollamaExe list 2>&1 | Out-Null
+    # 'ollama list' writes to stderr until the daemon is reachable; under
+    # $ErrorActionPreference='Stop' the 2>&1 merge surfaces that as a
+    # terminating NativeCommandError that would abort the whole install on
+    # the very first probe. Swallow it and rely on $LASTEXITCODE so the
+    # Start-Process serve fallback below actually runs (issue #522).
+    try { & $ollamaExe list 2>&1 | Out-Null } catch { }
     if ($LASTEXITCODE -eq 0) {
         $ollamaReady = $true
         break
     }
     if ($i -eq 5) {
-        # Daemon clearly isn't auto-running — start it ourselves. Ollama
+        # Daemon clearly isn't auto-running - start it ourselves. Ollama
         # for Windows uses the tray app `ollama app.exe`; falling back to
         # `ollama serve` works headless.
         Start-Process -FilePath $ollamaExe -ArgumentList 'serve' -WindowStyle Hidden -ErrorAction SilentlyContinue
@@ -354,11 +359,11 @@ for ($i = 0; $i -lt 60; $i++) {
     Start-Sleep -Seconds 1
 }
 if (-not $ollamaReady) {
-    Write-Warn2 "Ollama daemon didn't become ready in 60s. Continuing — bg-orchestrator will retry later."
+    Write-Warn2 "Ollama daemon didn't become ready in 60s. Continuing - bg-orchestrator will retry later."
 }
 
 # ---------------------------------------------------------------------------
-# 8. Pull a starter model (qwen3.5:2b — ~1.5 GB)
+# 8. Pull a starter model (qwen3.5:2b - ~1.5 GB)
 # ---------------------------------------------------------------------------
 
 $modelPullOk = $false
@@ -372,11 +377,11 @@ if ($ollamaReady) {
         Write-Warn2 "ollama pull failed; the bg-orchestrator will retry once Ollama is reachable."
     }
 } else {
-    Write-Warn2 "Skipping model pull — daemon wasn't ready."
+    Write-Warn2 "Skipping model pull - daemon wasn't ready."
 }
 
 # ---------------------------------------------------------------------------
-# 9. jarvis.cmd shim — so bare `jarvis` works in any new PowerShell
+# 9. jarvis.cmd shim - so bare `jarvis` works in any new PowerShell
 # ---------------------------------------------------------------------------
 
 $binDir = Join-Path $installRoot 'bin'
@@ -387,7 +392,7 @@ if (-not (Test-Path $binDir)) {
 }
 
 # %~dp0 in a .cmd file resolves to the directory containing the script,
-# so the shim is self-locating — moving %LOCALAPPDATA%\OpenJarvis won't
+# so the shim is self-locating - moving %LOCALAPPDATA%\OpenJarvis won't
 # break it as long as the user moves the whole tree. `uv` is resolved
 # from PATH at runtime (astral installer adds it to User PATH); avoids
 # pinning to the install-time uv.exe path which can shift on uv updates.
@@ -400,7 +405,7 @@ uv run --project "%SRC%" jarvis %*
 Set-Content -Path $shimPath -Value $shimContent -Encoding ASCII
 
 # Add %LOCALAPPDATA%\OpenJarvis\bin to User PATH if it isn't already
-# there. The current process won't see it until restart — handled in the
+# there. The current process won't see it until restart - handled in the
 # final banner.
 #
 # Compare against the EXPANDED form: a previous install may have written
@@ -430,7 +435,7 @@ Write-Ok "jarvis shim installed at $shimPath"
 $serviceScript = Join-Path $srcDir 'deploy\windows\jarvis-service.ps1'
 $shouldInstallService = $false
 
-# Pre-check admin if the user wants the service — Register-ScheduledTask
+# Pre-check admin if the user wants the service - Register-ScheduledTask
 # requires elevation. We do this before the prompt so we don't ask "do
 # you want the service?" only to fail with Access Denied after they say
 # yes.
@@ -439,7 +444,7 @@ $isAdmin = ([Security.Principal.WindowsPrincipal] `
 ).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)
 
 if ($Service -and -not $isAdmin) {
-    Write-Fail "-Service was requested, but this PowerShell is not elevated. Register-ScheduledTask needs admin rights — re-run from an elevated PowerShell, or drop -Service."
+    Write-Fail "-Service was requested, but this PowerShell is not elevated. Register-ScheduledTask needs admin rights - re-run from an elevated PowerShell, or drop -Service."
 }
 if ($Service) {
     $shouldInstallService = $true
@@ -448,7 +453,7 @@ if ($Service) {
 } elseif (-not $isAdmin) {
     # Default to skip-with-explanation when we can't elevate, rather
     # than prompting and then failing at Register-ScheduledTask.
-    Write-Warn2 "Skipping scheduled-task setup — this PowerShell is not elevated."
+    Write-Warn2 "Skipping scheduled-task setup - this PowerShell is not elevated."
     Write-Warn2 "  Register-ScheduledTask requires admin. To install the service later:"
     Write-Warn2 "    Right-click PowerShell -> Run as administrator, then run:"
     Write-Warn2 "    powershell -ExecutionPolicy Bypass -File `"$serviceScript`" install"
@@ -464,7 +469,7 @@ if ($Service) {
         $reply = Read-Host "Register OpenJarvis as a Windows scheduled task (auto-start at logon, loopback only)? [y/N]"
         $shouldInstallService = ($reply -match '^[yY]')
     } else {
-        Write-Warn2 "Non-interactive install — skipping scheduled-task setup."
+        Write-Warn2 "Non-interactive install - skipping scheduled-task setup."
         Write-Warn2 "To register the service later, run (from an elevated PowerShell):"
         Write-Warn2 "  powershell -ExecutionPolicy Bypass -File `"$serviceScript`" install"
     }
@@ -487,9 +492,9 @@ if ($shouldInstallService) {
 # ---------------------------------------------------------------------------
 
 Write-Host ""
-Write-Host "  ┌──────────────────────────────────┐" -ForegroundColor Green
-Write-Host "  │   OpenJarvis install complete    │" -ForegroundColor Green
-Write-Host "  └──────────────────────────────────┘" -ForegroundColor Green
+Write-Host "  +----------------------------------+" -ForegroundColor Green
+Write-Host "  |   OpenJarvis install complete    |" -ForegroundColor Green
+Write-Host "  +----------------------------------+" -ForegroundColor Green
 Write-Host ""
 Write-Host "  Repo:    $srcDir"
 

src/openjarvis/channels/_stubs.py

@@ -60,7 +60,27 @@ class BaseChannel(ABC):
         conversation_id: str = "",
         metadata: Dict[str, Any] | None = None,
     ) -> bool:
-        """Send a message to a specific channel. Returns True on success."""
+        """Send a message to a specific channel. Returns True on success.
+
+        Canonical send contract shared by **every** channel adapter:
+
+        ``channel``
+            The DESTINATION identifier — the per-adapter native id of the
+            place the message goes (Discord/Slack channel id, Telegram chat
+            id, email recipient address, ...).  This is *not* the channel
+            TYPE label.  An incoming :class:`ChannelMessage` carries that
+            destination in its ``conversation_id`` field (``channel`` there
+            is only the type label such as ``"discord"``), so dispatch code
+            replying to a message must pass ``cm.conversation_id`` here.
+        ``conversation_id``
+            An optional reply/thread reference — the native id of the
+            message being replied to (Discord ``message_reference``, Slack
+            ``thread_ts``, Telegram ``reply_to_message_id``, email
+            ``In-Reply-To``, ...).  When replying to an inbound message this
+            should be ``cm.message_id``, never the channel id.  Passing a
+            channel id here yields broken references (e.g. Discord
+            ``MESSAGE_REFERENCE_UNKNOWN_MESSAGE``).
+        """
 
     @abstractmethod
     def status(self) -> ChannelStatus:

src/openjarvis/channels/telegram.py

@@ -112,7 +112,15 @@ class TelegramChannel(BaseChannel):
 
             _TELEGRAM_MAX_LEN = 4096
             url = f"https://api.telegram.org/bot{self._token}/sendMessage"
-            chat_id = conversation_id or channel
+            # Canonical channel send contract (see BaseChannel.send): the first
+            # positional ``channel`` arg is the DESTINATION (the Telegram chat
+            # id).  ``conversation_id`` is the inbound message id used as a
+            # reply/thread reference (``reply_to_message_id``).  We fall back to
+            # ``conversation_id`` as the chat id only when ``channel`` is empty,
+            # for backwards compatibility with legacy callers that passed the
+            # chat id via ``conversation_id``.
+            chat_id = channel or conversation_id
+            reply_to = conversation_id if (channel and conversation_id) else ""
             chunks = textwrap.wrap(
                 content,
                 width=_TELEGRAM_MAX_LEN,
@@ -126,6 +134,8 @@ class TelegramChannel(BaseChannel):
                 }
                 if self._parse_mode:
                     payload["parse_mode"] = self._parse_mode
+                if reply_to:
+                    payload["reply_to_message_id"] = reply_to
 
                 resp = httpx.post(url, json=payload, timeout=10.0)
                 if resp.status_code >= 300:

src/openjarvis/cli/chat_cmd.py

@@ -28,12 +28,22 @@ def _read_input(prompt: str = "You> ") -> Optional[str]:
 @click.option("-a", "--agent", "agent_name", default=None, help="Agent type.")
 @click.option("--tools", default=None, help="Comma-separated tool names.")
 @click.option("--system", "system_prompt", default=None, help="Custom system prompt.")
+@click.option(
+    "--persona",
+    "persona_name",
+    default=None,
+    help=(
+        "Named persona dir under ~/.openjarvis/personas/<name>/ "
+        "(overrides config). Pass 'none' to disable all persona files."
+    ),
+)
 def chat(
     engine_key: str | None,
     model_name: str | None,
     agent_name: str | None,
     tools: str | None,
     system_prompt: str | None,
+    persona_name: str | None,
 ) -> None:
     """Start an interactive multi-turn chat session.
 
@@ -48,6 +58,14 @@ def chat(
 
     config = load_config()
 
+    import dataclasses as _dc
+
+    effective_mf = (
+        _dc.replace(config.memory_files, persona_name=persona_name)
+        if persona_name is not None
+        else config.memory_files
+    )
+
     # Resolve engine
     from openjarvis.engine import get_engine
     from openjarvis.intelligence import register_builtin_models
@@ -121,6 +139,21 @@ def chat(
 
                     kwargs["interactive"] = True
                     kwargs["confirm_callback"] = _confirm
+
+                import inspect as _inspect
+
+                if (
+                    "prompt_builder"
+                    in _inspect.signature(agent_cls.__init__).parameters
+                ):
+                    from openjarvis.prompt.builder import SystemPromptBuilder
+
+                    kwargs["prompt_builder"] = SystemPromptBuilder(
+                        agent_template=config.agent.default_system_prompt or "",
+                        memory_files_config=effective_mf,
+                        system_prompt_config=config.system_prompt,
+                    )
+
                 agent = agent_cls(engine, model, **kwargs)
         except Exception as exc:
             console.print(f"[yellow]Agent '{agent_key}' failed: {exc}[/yellow]")
@@ -147,6 +180,16 @@ def chat(
     _notifications = NotificationDispatcher(get_status())
 
     # Conversation state
+    if not system_prompt:
+        from openjarvis.prompt.builder import SystemPromptBuilder
+
+        builder = SystemPromptBuilder(
+            agent_template=config.agent.default_system_prompt or "",
+            memory_files_config=effective_mf,
+            system_prompt_config=config.system_prompt,
+        )
+        system_prompt = builder.build()
+
     history: List[Message] = []
     if system_prompt:
         history.append(Message(role=Role.SYSTEM, content=system_prompt))

src/openjarvis/cli/serve.py

@@ -261,6 +261,11 @@ def serve(
     # Resolve agent
     agent = None
     agent_key = agent_name or config.server.agent
+    # Tool instances resolved for the primary agent are reused below to build
+    # the scheduler's ToolExecutor — avoiding a second full SystemBuilder.build()
+    # (which would re-discover the engine, re-resolve tools, re-open the channel,
+    # etc.). See the scheduler block near the bottom of this function (#263).
+    resolved_tools: list = []
     if agent_key:
         try:
             import openjarvis.agents  # noqa: F401
@@ -328,6 +333,8 @@ def serve(
 
                     if tools:
                         agent_kwargs["tools"] = tools
+                    # Reuse these for the scheduler's ToolExecutor (#263).
+                    resolved_tools = tools
 
                 if getattr(agent_cls, "accepts_tools", False):
                     agent_kwargs["max_turns"] = config.agent.max_turns
@@ -461,6 +468,24 @@ def serve(
     # Create app
     from openjarvis.server.app import create_app
 
+    # Set up memory backend for context injection. Built before the scheduler
+    # block so the executor's JarvisSystem can reference it (#263).
+    memory_backend = None
+    if config.agent.context_from_memory:
+        try:
+            import openjarvis.tools.storage  # noqa: F401
+            from openjarvis.core.registry import MemoryRegistry
+
+            mem_key = config.memory.default_backend
+            if MemoryRegistry.contains(mem_key):
+                memory_backend = MemoryRegistry.create(
+                    mem_key,
+                    db_path=config.memory.db_path,
+                )
+                console.print("  Memory:    [cyan]active[/cyan]")
+        except Exception as exc:
+            logger.debug("Memory backend init failed: %s", exc)
+
     # Set up agent manager
     agent_manager = None
     if config.agent_manager.enabled:
@@ -500,9 +525,55 @@ def serve(
                 event_bus=bus,
                 trace_store=_trace_store,
             )
-            from openjarvis.system import SystemBuilder
+            # Reuse the components already built inline above instead of a
+            # second full SystemBuilder.build() — the original double-build
+            # re-discovered the engine, re-instrumented it, re-resolved tools,
+            # re-opened the channel and re-created the agent manager, costing
+            # ~30-40s on top of an already-paid startup (#263). The executor
+            # only reads engine/model/config/memory_backend/tool_executor/
+            # session_store/channel_backend from the system (see
+            # AgentExecutor), all of which are wired here.
+            from openjarvis.sessions.session import SessionStore
+            from openjarvis.system import JarvisSystem
+            from openjarvis.tools._stubs import ToolExecutor
 
-            system = SystemBuilder(config).build()
+            _sched_session_store = None
+            if config.sessions.enabled:
+                try:
+                    from pathlib import Path as _SchedPath
+
+                    _sched_session_store = SessionStore(
+                        db_path=_SchedPath(config.sessions.db_path).expanduser(),
+                        max_age_hours=config.sessions.max_age_hours,
+                        consolidation_threshold=(
+                            config.sessions.consolidation_threshold
+                        ),
+                    )
+                except Exception as exc:
+                    logger.debug("Scheduler session store init failed: %s", exc)
+
+            _sched_tool_executor = (
+                ToolExecutor(resolved_tools, bus) if resolved_tools else None
+            )
+
+            system = JarvisSystem(
+                config=config,
+                bus=bus,
+                engine=engine,
+                engine_key=engine_name,
+                model=model_name,
+                agent=agent,
+                agent_name=agent_key or "",
+                tools=resolved_tools,
+                tool_executor=_sched_tool_executor,
+                memory_backend=memory_backend,
+                telemetry_store=telem_store,
+                trace_store=_trace_store,
+                session_store=_sched_session_store,
+                capability_policy=sec.capability_policy,
+                agent_manager=agent_manager,
+                agent_executor=executor,
+            )
             executor.set_system(system)
 
             agent_scheduler = AgentScheduler(
@@ -522,23 +593,6 @@ def serve(
         except Exception as exc:
             logger.debug("Agent scheduler init failed: %s", exc)
 
-    # Set up memory backend for context injection
-    memory_backend = None
-    if config.agent.context_from_memory:
-        try:
-            import openjarvis.tools.storage  # noqa: F401
-            from openjarvis.core.registry import MemoryRegistry
-
-            mem_key = config.memory.default_backend
-            if MemoryRegistry.contains(mem_key):
-                memory_backend = MemoryRegistry.create(
-                    mem_key,
-                    db_path=config.memory.db_path,
-                )
-                console.print("  Memory:    [cyan]active[/cyan]")
-        except Exception as exc:
-            logger.debug("Memory backend init failed: %s", exc)
-
     # --- Channel Gateway: API key, sessions, ChannelBridge ---
     import os as _os
 

src/openjarvis/prompt/builder.py

@@ -229,6 +229,11 @@ class SystemPromptBuilder:
             )
 
     def _load_file(self, path_str: str, max_chars: int) -> str:
+        # An empty path means "no file" (e.g. the persona "none" opt-out, which
+        # resolves to empty paths). Guard before Path("") — which becomes "." —
+        # so reading it does not raise IsADirectoryError.
+        if not path_str:
+            return ""
         path = Path(path_str).expanduser()
         if not path.exists():
             return ""

src/openjarvis/server/api_routes.py

@@ -285,14 +285,39 @@ async def memory_config(request: Request):
 async def memory_index(req: MemoryIndexRequest, request: Request):
     """Index files from a path into memory."""
     try:
+        import os
         from pathlib import Path
 
+        from openjarvis.security.file_policy import is_sensitive_file
         from openjarvis.tools.storage.ingest import ingest_path
 
         target = Path(req.path).expanduser().resolve()
         if not target.exists():
             raise HTTPException(status_code=404, detail=f"Path not found: {req.path}")
 
+        # Sandbox: when workspace roots are configured via OPENJARVIS_WORKSPACE
+        # (os.pathsep-separated), only allow indexing inside them. This endpoint
+        # must not become an arbitrary-filesystem read primitive over the API.
+        workspace = os.environ.get("OPENJARVIS_WORKSPACE", "").strip()
+        if workspace:
+            roots = [
+                Path(d).expanduser().resolve()
+                for d in workspace.split(os.pathsep)
+                if d.strip()
+            ]
+            if not any(
+                target == root or root in target.parents for root in roots
+            ):
+                raise HTTPException(
+                    status_code=403,
+                    detail="Path is outside the allowed workspace directories.",
+                )
+        # Never ingest sensitive files (.env, private keys, credentials, ...).
+        if target.is_file() and is_sensitive_file(target):
+            raise HTTPException(
+                status_code=403, detail="Refusing to index a sensitive file."
+            )
+
         backend = _get_memory_backend(request)
         if backend is None:
             raise HTTPException(status_code=503, detail="Memory is not configured")

src/openjarvis/server/auth_middleware.py

@@ -33,7 +33,10 @@ class AuthMiddleware(BaseHTTPMiddleware):
                     status_code=401,
                 )
             scheme, _, token = auth.partition(" ")
-            if scheme.lower() != "bearer" or token != self._api_key:
+            # Constant-time comparison to avoid leaking the key via timing.
+            if scheme.lower() != "bearer" or not secrets.compare_digest(
+                token, self._api_key
+            ):
                 return JSONResponse(
                     {"detail": "Invalid API key"},
                     status_code=401,
@@ -42,8 +45,18 @@ class AuthMiddleware(BaseHTTPMiddleware):
 
     @staticmethod
     def _requires_auth(path: str) -> bool:
-        """Only protect API routes, not the frontend UI or static assets."""
-        return path.startswith("/v1/") or path.startswith("/api/")
+        """Protect API routes and operational metrics; leave the UI/health open.
+
+        ``/metrics`` exposes request/token counters that should not be readable
+        by unauthenticated clients, so it is gated alongside ``/v1`` and
+        ``/api``. ``/health`` stays open for liveness probes.
+        """
+        return (
+            path.startswith("/v1/")
+            or path.startswith("/api/")
+            or path == "/metrics"
+            or path.startswith("/metrics/")
+        )
 
 
 

src/openjarvis/server/webhook_routes.py

@@ -113,7 +113,14 @@ def create_webhook_router(
         signature = request.headers.get("X-Twilio-Signature", "")
         url = str(request.url)
 
-        if twilio_auth_token and not _validate_twilio_signature(
+        # Fail closed: an unconfigured token means we cannot verify the sender,
+        # so reject rather than trust unsigned input.
+        if not twilio_auth_token:
+            logger.error(
+                "Twilio webhook rejected: TWILIO_AUTH_TOKEN not configured."
+            )
+            return Response("Webhook signature verification not configured", 403)
+        if not _validate_twilio_signature(
             twilio_auth_token, url, params, signature
         ):
             return Response("Invalid signature", status_code=403)
@@ -257,7 +264,13 @@ def create_webhook_router(
         request: Request,
     ) -> Response:
         auth = request.headers.get("Authorization", "")
-        if bluebubbles_password and auth != bluebubbles_password:
+        # Fail closed when no password is configured.
+        if not bluebubbles_password:
+            logger.error(
+                "BlueBubbles webhook rejected: password not configured."
+            )
+            return Response("Webhook authentication not configured", 403)
+        if not hmac.compare_digest(auth, bluebubbles_password):
             return Response("Invalid password", status_code=403)
 
         payload = await request.json()
@@ -292,7 +305,11 @@ def create_webhook_router(
         token = request.query_params.get("hub.verify_token", "")
         challenge = request.query_params.get("hub.challenge", "")
 
-        if mode == "subscribe" and token == whatsapp_verify_token:
+        # Fail closed: never echo the challenge if no verify token is set,
+        # otherwise an empty token would match an empty query value.
+        if not whatsapp_verify_token:
+            return Response("Forbidden", status_code=403)
+        if mode == "subscribe" and hmac.compare_digest(token, whatsapp_verify_token):
             return PlainTextResponse(challenge)
         return Response("Forbidden", status_code=403)
 
@@ -302,19 +319,23 @@ def create_webhook_router(
     ) -> Response:
         body_bytes = await request.body()
 
-        # Verify signature
-        if whatsapp_app_secret:
-            signature = request.headers.get("X-Hub-Signature-256", "")
-            expected = (
-                "sha256="
-                + hmac.new(
-                    whatsapp_app_secret.encode(),
-                    body_bytes,
-                    hashlib.sha256,
-                ).hexdigest()
+        # Fail closed: reject when no app secret is configured to verify HMAC.
+        if not whatsapp_app_secret:
+            logger.error(
+                "WhatsApp webhook rejected: app secret not configured."
             )
-            if not hmac.compare_digest(signature, expected):
-                return Response("Invalid signature", status_code=403)
+            return Response("Webhook signature verification not configured", 403)
+        signature = request.headers.get("X-Hub-Signature-256", "")
+        expected = (
+            "sha256="
+            + hmac.new(
+                whatsapp_app_secret.encode(),
+                body_bytes,
+                hashlib.sha256,
+            ).hexdigest()
+        )
+        if not hmac.compare_digest(signature, expected):
+            return Response("Invalid signature", status_code=403)
 
         payload = json.loads(body_bytes)
         for entry in payload.get("entry", []):
@@ -349,16 +370,16 @@ def create_webhook_router(
         # Get the SendBlue channel — may be passed at init or set later
         sb = sendblue_channel or getattr(request.app.state, "sendblue_channel", None)
 
-        # Verify webhook secret if configured
-        if sb and sb.webhook_secret:
-            header_secret = request.headers.get("x-sendblue-secret", "")
-            if header_secret != sb.webhook_secret:
-                return Response("Invalid secret", status_code=403)
-        elif sb:
-            logger.warning(
-                "SendBlue webhook received without secret verification. "
-                "Set webhook_secret for HMAC validation."
+        # Fail closed: require a configured channel + webhook secret to verify
+        # the sender before processing any inbound message.
+        if sb is None or not getattr(sb, "webhook_secret", ""):
+            logger.error(
+                "SendBlue webhook rejected: webhook_secret not configured."
             )
+            return Response("Webhook secret not configured", status_code=403)
+        header_secret = request.headers.get("x-sendblue-secret", "")
+        if not hmac.compare_digest(header_secret, sb.webhook_secret):
+            return Response("Invalid secret", status_code=403)
 
         # Ignore outbound status callbacks
         if payload.get("is_outbound", False):

src/openjarvis/system/core.py

@@ -268,10 +268,20 @@ class JarvisSystem:
 
             if reply:
                 try:
+                    # Canonical channel send contract (see BaseChannel.send):
+                    # the first positional arg is the DESTINATION id, and the
+                    # `conversation_id=` kwarg is the inbound message id used as
+                    # a reply/thread reference.  ``cm.conversation_id`` holds the
+                    # real per-adapter destination (Discord/Slack channel id,
+                    # Telegram chat id, ...) while ``cm.channel`` is only the
+                    # channel TYPE label ("discord", "telegram", ...).  Passing
+                    # the type label as the destination produced HTTP 400s
+                    # (#515) and using the channel id as a reply reference
+                    # produced MESSAGE_REFERENCE_UNKNOWN_MESSAGE (#516).
                     channel_bridge.send(
-                        cm.channel,
+                        cm.conversation_id,
                         reply,
-                        conversation_id=cm.conversation_id,
+                        conversation_id=getattr(cm, "message_id", ""),
                     )
                 except Exception:
                     logger.exception("Channel send error")

src/openjarvis/tools/http_request.py

@@ -5,6 +5,7 @@ from __future__ import annotations
 import logging
 import os
 import time
+import urllib.parse
 from typing import Any
 
 import httpx
@@ -21,6 +22,13 @@ _MAX_RESPONSE_BYTES = 1_048_576
 
 _ALLOWED_METHODS = frozenset({"GET", "POST", "PUT", "DELETE", "PATCH", "HEAD"})
 
+# Cap redirect chains so a malicious server cannot loop us indefinitely.
+_MAX_REDIRECTS = 5
+
+
+class _SSRFRedirectError(Exception):
+    """Raised when a redirect target fails the SSRF check."""
+
 
 @ToolRegistry.register("http_request")
 class HttpRequestTool(BaseTool):
@@ -136,13 +144,11 @@ class HttpRequestTool(BaseTool):
 
         try:
             t0 = time.time()
-            response = httpx.request(
-                method,
-                url,
-                headers=headers,
-                content=body,
-                timeout=float(timeout),
-                follow_redirects=True,
+            # Follow redirects manually so each hop is re-checked for SSRF — an
+            # allowed public URL must not be able to 30x-redirect us to an
+            # internal/metadata address.
+            response = self._request_following_redirects(
+                method, url, headers=headers, content=body, timeout=float(timeout)
             )
             elapsed_ms = (time.time() - t0) * 1000
 
@@ -178,6 +184,12 @@ class HttpRequestTool(BaseTool):
                 content=f"Request timed out after {timeout}s: {exc}",
                 success=False,
             )
+        except _SSRFRedirectError as exc:
+            return ToolResult(
+                tool_name="http_request",
+                content=f"SSRF protection blocked redirect: {exc}",
+                success=False,
+            )
         except httpx.RequestError as exc:
             return ToolResult(
                 tool_name="http_request",
@@ -191,5 +203,52 @@ class HttpRequestTool(BaseTool):
                 success=False,
             )
 
+    @staticmethod
+    def _request_following_redirects(
+        method: str,
+        url: str,
+        *,
+        headers: dict,
+        content: Any,
+        timeout: float,
+    ) -> httpx.Response:
+        """Issue the request, re-checking SSRF on every redirect hop.
+
+        httpx's built-in ``follow_redirects`` would chase a 30x ``Location``
+        without re-validating it, letting a public URL bounce us to an internal
+        host. We follow manually and run :func:`check_ssrf` on each target.
+        """
+        current_url = url
+        current_method = method
+        body = content
+        # Use module-level ``httpx.request`` (not a private Client) so the SSRF
+        # re-check seam stays patchable by callers' tests, with redirects
+        # disabled so we control every hop ourselves.
+        for _ in range(_MAX_REDIRECTS + 1):
+            response = httpx.request(
+                current_method,
+                current_url,
+                headers=headers,
+                content=body,
+                timeout=timeout,
+                follow_redirects=False,
+            )
+            if response.status_code not in (301, 302, 303, 307, 308):
+                return response
+            location = response.headers.get("location", "")
+            if not location:
+                return response
+            # Resolve relative redirects against the URL we just fetched.
+            current_url = urllib.parse.urljoin(str(response.url), location)
+            ssrf_error = check_ssrf(current_url)
+            if ssrf_error:
+                raise _SSRFRedirectError(ssrf_error)
+            # Per RFC 7231, 301/302/303 turn the method into GET and drop
+            # the body (except for HEAD).
+            if response.status_code in (301, 302, 303) and current_method != "HEAD":
+                current_method = "GET"
+                body = None
+        raise _SSRFRedirectError(f"Exceeded maximum of {_MAX_REDIRECTS} redirects.")
+
 
 __all__ = ["HttpRequestTool"]

tests/channels/test_discord_channel.py

@@ -130,3 +130,63 @@ class TestStatus:
         ch = DiscordChannel()
         ch.connect()
         assert ch.status() == ChannelStatus.ERROR
+
+
+class TestWireChannelEndToEnd:
+    """Regression for #515/#516 — the full inbound→reply path through
+    JarvisSystem.wire_channel must call the real Discord REST API with the
+    numeric channel id (not "discord") and a message_reference equal to the
+    inbound message id (not the channel id).
+    """
+
+    def test_reply_hits_real_channel_id_and_message_reference(self, tmp_path):
+        from openjarvis.channels._stubs import ChannelMessage
+        from openjarvis.core.config import JarvisConfig
+        from openjarvis.core.events import EventBus
+        from openjarvis.system import JarvisSystem
+
+        config = JarvisConfig()
+        config.sessions.db_path = str(tmp_path / "sessions.db")
+        from unittest.mock import MagicMock as _MM
+
+        system = JarvisSystem(
+            config=config,
+            bus=EventBus(record_history=False),
+            engine=_MM(),
+            engine_key="mock",
+            model="test-model",
+            agent_name="",
+        )
+        system.ask = _MM(return_value={"content": "pong"})
+
+        channel = DiscordChannel(bot_token="my-bot-token")
+        system.wire_channel(channel)
+
+        # Exactly the ChannelMessage shape DiscordChannel._gateway_loop emits:
+        # channel = "discord" (TYPE label), conversation_id = numeric channel
+        # id, message_id = numeric message id.
+        cm = ChannelMessage(
+            channel="discord",
+            sender="user-1",
+            content="hello",
+            message_id="111122223333444455",
+            conversation_id="987654321098765432",
+        )
+
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        with patch("httpx.post", return_value=mock_response) as mock_post:
+            # Invoke the handler wire_channel registered on the channel.
+            for handler in channel._handlers:
+                handler(cm)
+
+        mock_post.assert_called_once()
+        url = mock_post.call_args[0][0]
+        # #515: destination is the numeric channel id, not the "discord" label.
+        assert "discord.com/api/v10/channels/987654321098765432/messages" in url
+        assert "channels/discord/messages" not in url
+        payload = mock_post.call_args[1]["json"]
+        assert payload["content"] == "pong"
+        # #516: message_reference is the inbound message id, NOT the channel id.
+        assert payload["message_reference"] == {"message_id": "111122223333444455"}
+        assert payload["message_reference"]["message_id"] != "987654321098765432"

tests/channels/test_telegram.py

@@ -105,6 +105,41 @@ class TestSend:
         event_types = [e.event_type for e in bus.history]
         assert EventType.CHANNEL_MESSAGE_SENT in event_types
 
+    def test_send_uses_channel_as_chat_id_under_unified_contract(self):
+        """Canonical contract (#515/#516): the first positional ``channel``
+        arg is the chat destination, and ``conversation_id`` is the inbound
+        message id used as ``reply_to_message_id`` — not the chat id."""
+        ch = TelegramChannel(bot_token="123:ABC")
+
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+
+        with patch("httpx.post", return_value=mock_response) as mock_post:
+            result = ch.send("12345678", "Reply!", conversation_id="55")
+            assert result is True
+            payload = mock_post.call_args[1]["json"]
+            # Destination is the chat id from the positional channel arg.
+            assert payload["chat_id"] == "12345678"
+            # conversation_id becomes the reply reference, not the chat id.
+            assert payload["reply_to_message_id"] == "55"
+
+    def test_send_legacy_conversation_id_only_still_targets_chat(self):
+        """Backwards compatibility: a legacy caller passing the chat id via
+        ``conversation_id`` (with an empty ``channel``) still delivers."""
+        ch = TelegramChannel(bot_token="123:ABC")
+
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+
+        with patch("httpx.post", return_value=mock_response) as mock_post:
+            result = ch.send("", "Hello!", conversation_id="12345678")
+            assert result is True
+            payload = mock_post.call_args[1]["json"]
+            assert payload["chat_id"] == "12345678"
+            # When channel is empty, conversation_id is the chat id, so it must
+            # not also be used as a self-referential reply id.
+            assert "reply_to_message_id" not in payload
+
 
 class TestStatus:
     def test_no_token_connect_error(self):

tests/cli/test_serve_channel_wiring.py

@@ -79,10 +79,14 @@ class TestWireChannelWithAgent:
 
         system.ask.assert_called_once()
         assert system.ask.call_args[0][0] == "ping"
+        # Canonical send contract (#515/#516): the destination is the real
+        # per-adapter id (carried in ChannelMessage.conversation_id), not the
+        # channel TYPE label, and the conversation_id kwarg is the inbound
+        # message id used as a reply reference, not the channel id.
         mock_channel.send.assert_called_once_with(
-            "telegram",
+            "42",
             "pong",
-            conversation_id="42",
+            conversation_id="1",
         )
 
     def test_session_store_created_lazily(self, tmp_path):
@@ -126,13 +130,101 @@ class TestWireChannelWithEngine:
         handler = mock_channel.on_message.call_args[0][0]
         handler(_make_channel_message(content="hi"))
 
+        # Canonical send contract (#515/#516): destination = real channel id
+        # (from ChannelMessage.conversation_id), reply ref = inbound message id.
         mock_channel.send.assert_called_once_with(
-            "telegram",
+            "42",
             "raw reply",
-            conversation_id="42",
+            conversation_id="1",
         )
 
 
+class TestWireChannelCanonicalContract:
+    """Regression for #515/#516 — wire_channel must dispatch the canonical
+    send contract so each adapter receives the right destination/reply ids,
+    regardless of the channel TYPE label.
+    """
+
+    def test_discord_uses_real_channel_id_not_type_label(self, tmp_path):
+        """A Discord ChannelMessage (channel="discord" TYPE label,
+        conversation_id=<numeric channel id>, message_id=<numeric msg id>)
+        must reply to the numeric channel id, with the message id as the
+        reply reference — never "discord" as destination (#515) and never the
+        channel id as the message reference (#516).
+        """
+        system = _make_system(tmp_path=tmp_path)
+        system.ask = MagicMock(return_value={"content": "pong"})
+
+        mock_channel = MagicMock()
+        system.wire_channel(mock_channel)
+        handler = mock_channel.on_message.call_args[0][0]
+
+        cm = ChannelMessage(
+            channel="discord",
+            sender="user-1",
+            content="hello",
+            message_id="111122223333444455",
+            conversation_id="987654321098765432",
+        )
+        handler(cm)
+
+        args, kwargs = mock_channel.send.call_args
+        # Destination is the real Discord channel id, NOT the type label.
+        assert args[0] == "987654321098765432"
+        assert args[0] != "discord"
+        # Reply reference is the inbound message id, NOT the channel id.
+        assert kwargs["conversation_id"] == "111122223333444455"
+        assert kwargs["conversation_id"] != "987654321098765432"
+
+    def test_telegram_uses_chat_id_and_message_id(self, tmp_path):
+        """Telegram must keep working under the unified contract: destination
+        is the chat id (conversation_id), reply ref is the message id."""
+        system = _make_system(tmp_path=tmp_path)
+        system.ask = MagicMock(return_value={"content": "pong"})
+
+        mock_channel = MagicMock()
+        system.wire_channel(mock_channel)
+        handler = mock_channel.on_message.call_args[0][0]
+
+        cm = ChannelMessage(
+            channel="telegram",
+            sender="42",
+            content="ping",
+            message_id="55",
+            conversation_id="12345678",
+        )
+        handler(cm)
+
+        mock_channel.send.assert_called_once_with(
+            "12345678",
+            "pong",
+            conversation_id="55",
+        )
+
+    def test_session_key_still_uses_conversation_id(self, tmp_path):
+        """The fix must not change session isolation keying, which still uses
+        ``<channel>:<conversation_id>``."""
+        system = _make_system(tmp_path=tmp_path)
+        system.ask = MagicMock(return_value={"content": "ok"})
+
+        mock_channel = MagicMock()
+        system.wire_channel(mock_channel)
+        handler = mock_channel.on_message.call_args[0][0]
+
+        cm = ChannelMessage(
+            channel="discord",
+            sender="u1",
+            content="hi",
+            message_id="msg-9",
+            conversation_id="chan-7",
+        )
+        handler(cm)
+
+        # Session was created under the channel:conversation_id key.
+        session = system.session_store.get_or_create("discord:chan-7")
+        assert any(m.content == "hi" for m in session.messages)
+
+
 class TestWireChannelSessionIsolation:
     """Separate conversation_ids get independent sessions."""
 

tests/cli/test_serve_single_build.py

@@ -0,0 +1,210 @@
+"""Regression tests for #263 — ``jarvis serve`` must build the system once.
+
+serve.py used to construct all heavy components inline and then call
+``SystemBuilder(config).build()`` a second time inside the scheduler block,
+re-discovering the engine, re-instrumenting it, re-resolving tools, re-opening
+the channel and re-creating the agent manager — ~30-40s of redundant work.
+
+These tests pin the fix:
+
+1. ``SystemBuilder.build`` is never called during ``jarvis serve`` startup
+   (the duplicate build is gone).
+2. The ``AgentExecutor`` still receives a system exposing the attributes it
+   actually reads: ``tool_executor``, ``session_store``, ``memory_backend``,
+   plus ``engine`` / ``model`` / ``config``.
+"""
+
+from __future__ import annotations
+
+import importlib
+from unittest.mock import MagicMock, patch
+
+import pytest
+from click.testing import CliRunner
+
+from openjarvis.cli import cli
+
+pytest.importorskip("fastapi")
+pytest.importorskip("uvicorn")
+
+# ``openjarvis.cli.serve`` as an attribute resolves to the click *command*
+# (re-exported on the package); grab the real module to monkeypatch its globals.
+serve_mod = importlib.import_module("openjarvis.cli.serve")
+
+
+def _fake_engine() -> MagicMock:
+    engine = MagicMock()
+    engine.list_models.return_value = ["test-model"]
+    engine.health.return_value = True
+    engine.name = "mock"
+    return engine
+
+
+def _repopulate_registries() -> None:
+    """Re-run the @register decorators wiped by the autouse conftest fixture.
+
+    The tool/memory modules are import-cached, so a plain ``import`` inside
+    serve.py is a no-op after the registries are cleared per-test. Reload the
+    individual submodules so ToolRegistry/MemoryRegistry are populated exactly
+    as they would be on a fresh process — otherwise serve would resolve an
+    empty tool list and no memory backend, masking the very wiring under test.
+    """
+    import importlib
+    import sys
+
+    import openjarvis.agents  # noqa: F401
+    import openjarvis.tools  # noqa: F401
+    import openjarvis.tools.storage  # noqa: F401
+    from openjarvis.core.registry import (
+        AgentRegistry,
+        MemoryRegistry,
+        ToolRegistry,
+    )
+
+    if not AgentRegistry.keys():
+        for mod_name in list(sys.modules):
+            if mod_name.startswith("openjarvis.agents.") and not mod_name.endswith(
+                "_stubs"
+            ):
+                try:
+                    importlib.reload(sys.modules[mod_name])
+                except Exception:
+                    pass
+
+    if not ToolRegistry.keys():
+        for mod_name in list(sys.modules):
+            if (
+                mod_name.startswith("openjarvis.tools.")
+                and not mod_name.endswith("_stubs")
+                and not mod_name.endswith("agent_tools")
+            ):
+                try:
+                    importlib.reload(sys.modules[mod_name])
+                except Exception:
+                    pass
+
+    if not MemoryRegistry.keys():
+        for mod_name in list(sys.modules):
+            if mod_name.startswith(
+                "openjarvis.tools.storage."
+            ) and not mod_name.endswith("_stubs"):
+                try:
+                    importlib.reload(sys.modules[mod_name])
+                except Exception:
+                    pass
+
+
+def _run_serve(tmp_path, monkeypatch, *, build_spy, set_system_spy):
+    """Invoke ``jarvis serve`` with all heavy/blocking pieces stubbed out.
+
+    Returns the CliRunner result. The server is never actually started
+    (``uvicorn.run`` is a no-op) and no real engine is contacted.
+    """
+    from openjarvis.core.config import JarvisConfig
+
+    _repopulate_registries()
+
+    config = JarvisConfig()
+    # Keep the scheduler block alive (it owns the executor wiring under test)
+    # while pointing every store at the temp dir.
+    config.agent_manager.enabled = True
+    config.agent_manager.db_path = str(tmp_path / "agents.db")
+    config.sessions.enabled = True
+    config.sessions.db_path = str(tmp_path / "sessions.db")
+    config.memory.db_path = str(tmp_path / "memory.db")
+    config.telemetry.enabled = False
+    config.traces.enabled = False
+    config.channel.enabled = False
+    config.skills.enabled = False
+    config.server.host = "127.0.0.1"
+    config.server.port = 8123
+    # Resolve a model without contacting a real engine / discovery.
+    config.intelligence.default_model = "test-model"
+
+    engine = _fake_engine()
+
+    monkeypatch.setattr(serve_mod, "load_config", lambda *a, **k: config)
+    monkeypatch.setattr(serve_mod, "get_engine", lambda *a, **k: ("mock", engine))
+    monkeypatch.setattr(serve_mod, "discover_engines", lambda *a, **k: {})
+    monkeypatch.setattr(serve_mod, "discover_models", lambda *a, **k: {})
+
+    # setup_security returns its own context; pass the engine straight through
+    # so we don't need real guardrails wired up.
+    sec = MagicMock()
+    sec.engine = engine
+    sec.capability_policy = None
+    sec.audit_logger = None
+    monkeypatch.setattr("openjarvis.security.setup_security", lambda *a, **k: sec)
+
+    with (
+        patch(
+            "openjarvis.system.builder.SystemBuilder.build",
+            build_spy,
+        ),
+        patch(
+            "openjarvis.agents.executor.AgentExecutor.set_system",
+            set_system_spy,
+        ),
+        patch("uvicorn.run", lambda *a, **k: None),
+    ):
+        return CliRunner().invoke(cli, ["serve"], catch_exceptions=False)
+
+
+def test_serve_does_not_call_systembuilder_build(tmp_path, monkeypatch):
+    """The redundant second full build is gone (#263)."""
+    build_spy = MagicMock(
+        side_effect=AssertionError(
+            "SystemBuilder.build() must not run during `jarvis serve` startup "
+            "— it is the duplicate build #263 removed."
+        )
+    )
+    set_system_spy = MagicMock()
+
+    result = _run_serve(
+        tmp_path,
+        monkeypatch,
+        build_spy=build_spy,
+        set_system_spy=set_system_spy,
+    )
+
+    assert result.exit_code == 0, result.output
+    build_spy.assert_not_called()
+
+
+def test_executor_receives_required_system_attrs(tmp_path, monkeypatch):
+    """The executor still gets a system exposing the attributes it reads.
+
+    AgentExecutor reads engine/model/config/memory_backend/tool_executor/
+    session_store off ``self._system``; the de-dup must not strip any of them.
+    """
+    build_spy = MagicMock()
+    captured: dict = {}
+
+    def _capture_set_system(self, system):  # noqa: ANN001
+        captured["system"] = system
+        # Preserve real behaviour so the executor is usable afterwards.
+        self._system = system
+
+    result = _run_serve(
+        tmp_path,
+        monkeypatch,
+        build_spy=build_spy,
+        set_system_spy=_capture_set_system,
+    )
+
+    assert result.exit_code == 0, result.output
+    # Built once, from the inline components — not via SystemBuilder.build().
+    build_spy.assert_not_called()
+
+    system = captured.get("system")
+    assert system is not None, "executor.set_system was never called"
+
+    # Correctness constraint from the verifier: these must survive the de-dup.
+    assert system.tool_executor is not None
+    assert system.session_store is not None
+    assert system.memory_backend is not None
+
+    # And the basics the executor resolves engine/model from.
+    assert system.engine is not None
+    assert system.model == "test-model"
+    assert system.config is not None

tests/prompt/test_persona_scope.py

@@ -31,3 +31,23 @@ def test_named_persona_resolves_to_personas_dir():
 def test_path_traversal_rejected(bad):
     with pytest.raises(ValueError):
         SystemPromptBuilder._resolve_persona(MemoryFilesConfig(persona_name=bad))
+
+
+def test_none_persona_build_does_not_raise():
+    """Regression (#497): `--persona none` resolves to empty file paths; building
+    the prompt must not raise IsADirectoryError when those empty paths are read
+    (Path("") is "." — reading a directory raised before the empty-path guard).
+    """
+    import dataclasses
+
+    from openjarvis.core.config import load_config
+
+    cfg = load_config()
+    mf = dataclasses.replace(cfg.memory_files, persona_name="none")
+    builder = SystemPromptBuilder(
+        agent_template=cfg.agent.default_system_prompt or "",
+        memory_files_config=mf,
+        system_prompt_config=cfg.system_prompt,
+    )
+    out = builder.build()
+    assert isinstance(out, str)

tests/server/test_auth_middleware.py

@@ -28,6 +28,10 @@ def _make_app(api_key: str) -> FastAPI:
     async def twilio_webhook():
         return {"status": "received"}
 
+    @app.get("/metrics")
+    async def metrics():
+        return {"requests": 0}
+
     return app
 
 
@@ -65,7 +69,18 @@ class TestAuthMiddleware:
         resp = client.post("/webhooks/twilio")
         assert resp.status_code == 200
 
+    def test_metrics_requires_auth(self, client):
+        resp = client.get("/metrics")
+        assert resp.status_code == 401
+
+    def test_metrics_accepts_valid_key(self, client):
+        resp = client.get(
+            "/metrics", headers={"Authorization": "Bearer oj_sk_test123"}
+        )
+        assert resp.status_code == 200
+
     def test_no_key_configured_allows_all(self):
         client = TestClient(_make_app(""))
         resp = client.get("/v1/models")
         assert resp.status_code == 200
+        assert client.get("/metrics").status_code == 200

tests/server/test_sendblue_webhook.py

@@ -41,6 +41,9 @@ def sendblue_channel():
         api_key_id="test_key",
         api_secret_key="test_secret",
         from_number="+15551234567",
+        # Webhooks now fail closed without a secret, so configure one and have
+        # the test client send the matching header by default.
+        webhook_secret="testsecret",
     )
     ch.connect()
     return ch
@@ -61,7 +64,9 @@ def webhook_app(mock_bridge, sendblue_channel):
 
 @pytest.fixture
 def client(webhook_app):
-    return TestClient(webhook_app)
+    # Send the webhook secret by default so message-handling tests reach the
+    # bridge; fail-closed behavior is covered separately below.
+    return TestClient(webhook_app, headers={"x-sendblue-secret": "testsecret"})
 
 
 # ---------------------------------------------------------------------------
@@ -169,7 +174,7 @@ class TestSendBlueWebhook:
         app = FastAPI()
         router = create_webhook_router(bridge=None, sendblue_channel=sendblue_channel)
         app.include_router(router)
-        c = TestClient(app)
+        c = TestClient(app, headers={"x-sendblue-secret": "testsecret"})
 
         resp = c.post(
             "/webhooks/sendblue",
@@ -181,6 +186,27 @@ class TestSendBlueWebhook:
         )
         assert resp.status_code == 200
 
+    def test_no_secret_configured_is_rejected(self, mock_bridge):
+        """Fail closed: a channel without a webhook_secret rejects all posts."""
+        from openjarvis.channels.sendblue import SendBlueChannel
+        from openjarvis.server.webhook_routes import create_webhook_router
+
+        ch = SendBlueChannel(
+            api_key_id="k", api_secret_key="s", from_number="+1555"
+        )
+        ch.connect()
+        app = FastAPI()
+        router = create_webhook_router(bridge=mock_bridge, sendblue_channel=ch)
+        app.include_router(router)
+        c = TestClient(app)
+
+        resp = c.post(
+            "/webhooks/sendblue",
+            json={"from_number": "+19127130720", "content": "Hi", "is_outbound": False},
+        )
+        assert resp.status_code == 403
+        mock_bridge.handle_incoming.assert_not_called()
+
 
 # ---------------------------------------------------------------------------
 # Health endpoint (requires agent_manager_routes)

tests/server/test_webhook_routes.py

@@ -213,3 +213,48 @@ class TestWhatsAppWebhook:
             },
         )
         assert resp.status_code == 200
+
+
+class TestWebhooksFailClosed:
+    """When a channel's secret/token is unset, webhooks must reject (403)."""
+
+    def _client(self, mock_bridge, **kwargs):
+        app = FastAPI()
+        app.include_router(create_webhook_router(bridge=mock_bridge, **kwargs))
+        return TestClient(app)
+
+    def test_twilio_without_token_rejected(self, mock_bridge):
+        c = self._client(mock_bridge)  # no twilio_auth_token
+        resp = c.post(
+            "/webhooks/twilio",
+            data={"From": "+15551234567", "Body": "hi", "MessageSid": "SM1"},
+        )
+        assert resp.status_code == 403
+        mock_bridge.handle_incoming.assert_not_called()
+
+    def test_bluebubbles_without_password_rejected(self, mock_bridge):
+        c = self._client(mock_bridge)  # no bluebubbles_password
+        resp = c.post(
+            "/webhooks/bluebubbles",
+            json={"type": "new-message", "data": {}},
+            headers={"Authorization": "anything"},
+        )
+        assert resp.status_code == 403
+
+    def test_whatsapp_without_secret_rejected(self, mock_bridge):
+        c = self._client(mock_bridge)  # no whatsapp_app_secret
+        resp = c.post(
+            "/webhooks/whatsapp",
+            content=b"{}",
+            headers={"Content-Type": "application/json"},
+        )
+        assert resp.status_code == 403
+
+    def test_whatsapp_verify_without_token_rejected(self, mock_bridge):
+        c = self._client(mock_bridge)  # no whatsapp_verify_token
+        resp = c.get(
+            "/webhooks/whatsapp",
+            params={"hub.mode": "subscribe", "hub.verify_token": "",
+                    "hub.challenge": "x"},
+        )
+        assert resp.status_code == 403

tests/tools/test_http_request.py

@@ -184,8 +184,9 @@ class TestHttpRequestTool:
         """Timeout should produce a clear error."""
         tool = HttpRequestTool()
         with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
-            with patch(
-                "openjarvis.tools.http_request.httpx.request",
+            with patch.object(
+                HttpRequestTool,
+                "_request_following_redirects",
                 side_effect=httpx.TimeoutException("timed out"),
             ):
                 result = tool.execute(url="https://slow.example.com", timeout=5)
@@ -196,14 +197,50 @@ class TestHttpRequestTool:
         """Connection error should produce a clear error."""
         tool = HttpRequestTool()
         with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
-            with patch(
-                "openjarvis.tools.http_request.httpx.request",
+            with patch.object(
+                HttpRequestTool,
+                "_request_following_redirects",
                 side_effect=httpx.ConnectError("Connection refused"),
             ):
                 result = tool.execute(url="https://down.example.com")
         assert result.success is False
         assert "Request error" in result.content
 
+    @respx.mock
+    def test_redirect_to_private_ip_blocked(self):
+        """A redirect to an internal/metadata host must be re-checked + blocked."""
+        respx.get("https://public.example.com/start").mock(
+            return_value=httpx.Response(
+                302, headers={"location": "http://169.254.169.254/latest/"}
+            )
+        )
+        tool = HttpRequestTool()
+        # First check (initial URL) passes; the redirect target is blocked.
+        with patch(
+            "openjarvis.tools.http_request.check_ssrf",
+            side_effect=[None, "Blocked host: 169.254.169.254"],
+        ):
+            result = tool.execute(url="https://public.example.com/start")
+        assert result.success is False
+        assert "SSRF protection blocked redirect" in result.content
+
+    @respx.mock
+    def test_safe_redirect_is_followed(self):
+        """A redirect to another public URL is followed normally."""
+        respx.get("https://public.example.com/start").mock(
+            return_value=httpx.Response(
+                302, headers={"location": "https://public.example.com/final"}
+            )
+        )
+        respx.get("https://public.example.com/final").mock(
+            return_value=httpx.Response(200, text="done")
+        )
+        tool = HttpRequestTool()
+        with patch("openjarvis.tools.http_request.check_ssrf", return_value=None):
+            result = tool.execute(url="https://public.example.com/start")
+        assert result.success is True
+        assert "done" in result.content
+
     def test_method_validation(self):
         """Invalid HTTP method should be rejected."""
         tool = HttpRequestTool()