Compare commits

...
Author SHA1 Message Date
Elliot Slusky c2e1c375aa Merge pull request #679 from gilbert-barajas/fix/serve-persona-prompt-builder
fix: served/SDK agents silently lose their persona (SOUL.md never reaches the model over HTTP)
2026-08-14 17:16:19 -07:00
Elliot Slusky ef28e5f84b fix: complete persona wiring across agent entry points 2026-08-14 17:11:25 -07:00
Ari f2483e7bf3 fix(frontend): move MessageBubble hooks above the early return
The three useMemo calls ran after the isUser early return, so they
were skipped entirely for user messages, violating React's rules of
hooks. Caught by the react-hooks/rules-of-hooks lint rule added last
commit. Pure reordering, no logic change; tsc and the full vitest
suite (38/38) still pass.
2026-08-14 17:05:00 -07:00
Elliot Slusky 156d41d2f9 Merge remote-tracking branch 'origin/main' into elliot/pr679-fix 2026-08-14 17:02:52 -07:00
Elliot Slusky 4b7bb936ff fix(desktop): complete remote server support
Allow plaintext user-configured API hosts in the macOS webview and authenticate remote agent event WebSockets with the configured API key. Add regression coverage for both paths.
2026-08-14 16:29:59 -07:00
Haotian Zheng 3c68a17ac5 fix(desktop): allow remote API connections 2026-08-14 16:29:59 -07:00
Gilbert Barajas 6e40d87eb5 fix: wire persona prompt_builder into the serve + SDK agent paths
cli/serve.py and sdk.py constructed their agents without passing
prompt_builder, so an agent reached over HTTP (jarvis serve) or via the
SDK silently lost its SOUL.md / MEMORY.md / USER.md persona while the
same agent via `jarvis ask` / `jarvis chat` kept it. cli/ask.py,
cli/chat_cmd.py, and the managed-agent executor already wired the
builder; these two entry points never did.

Found deploying a personal assistant: the server answered as a generic
assistant that explicitly denied being the persona, with SOUL.md
sitting correctly on disk the whole time. No error, no warning.

Mirrors the existing inspect.signature-guarded wiring from ask.py, so
agents whose __init__ doesn't accept the kwarg (e.g. OrchestratorAgent)
opt out automatically and keep their own system-prompt machinery.

Adds a serve-path regression test (tests/cli/test_serve_persona.py)
that fails without the fix: agent._prompt_builder is None on the
unpatched serve path, so the persona files never reach the model.
2026-07-27 00:50:56 -05:00
14 changed files with 341 additions and 42 deletions
+14
View File
@@ -0,0 +1,14 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>NSAppTransportSecurity</key>
<dict>
<!-- The desktop API URL is user-configurable, so it cannot be represented
by Tauri's single, build-time exceptionDomain setting. Keep this
exception scoped to WKWebView; native URLSession traffic retains ATS. -->
<key>NSAllowsArbitraryLoadsInWebContent</key>
<true/>
</dict>
</dict>
</plist>
+1 -2
View File
@@ -24,7 +24,7 @@
}
],
"security": {
"csp": "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:*; img-src 'self' data: blob:"
"csp": "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; connect-src 'self' http: https: ws: wss:; img-src 'self' data: blob:"
}
},
"bundle": {
@@ -45,7 +45,6 @@
"macOS": {
"entitlements": "Entitlements.plist",
"minimumSystemVersion": "10.15",
"exceptionDomain": "",
"frameworks": [],
"providerShortName": null,
"signingIdentity": "-"
+18 -18
View File
@@ -103,6 +103,24 @@ function CopyMessageButton({ content }: { content: string }) {
export function MessageBubble({ message, isLive = false }: Props) {
const isUser = message.role === 'user';
const cleanContent = useMemo(() => stripThinkTags(message.content), [message.content]);
// Build a ref→source lookup once per render. Memoized so the rehype plugin
// identity stays stable until the source list actually changes.
const sourcesMap = useMemo(() => {
const m = new Map<number, NonNullable<ChatMessage['researchSources']>[number]>();
for (const s of message.researchSources ?? []) {
if (typeof s.ref === 'number') m.set(s.ref, s);
}
return m;
}, [message.researchSources]);
const rehypePlugins = useMemo(() => {
const base: any[] = [[rehypeHighlight, { detect: true }], rehypeKatex];
if (sourcesMap.size > 0) base.push([rehypeCitations, { sources: sourcesMap }]);
return base;
}, [sourcesMap]);
if (isUser) {
return (
<div className="flex justify-end mb-4">
@@ -122,24 +140,6 @@ export function MessageBubble({ message, isLive = false }: Props) {
);
}
const cleanContent = useMemo(() => stripThinkTags(message.content), [message.content]);
// Build a ref→source lookup once per render. Memoized so the rehype plugin
// identity stays stable until the source list actually changes.
const sourcesMap = useMemo(() => {
const m = new Map<number, NonNullable<ChatMessage['researchSources']>[number]>();
for (const s of message.researchSources ?? []) {
if (typeof s.ref === 'number') m.set(s.ref, s);
}
return m;
}, [message.researchSources]);
const rehypePlugins = useMemo(() => {
const base: any[] = [[rehypeHighlight, { detect: true }], rehypeKatex];
if (sourcesMap.size > 0) base.push([rehypeCitations, { sources: sourcesMap }]);
return base;
}, [sourcesMap]);
return (
<div className="group mb-6">
{/* Deep Research timeline (steps + status) */}
+66
View File
@@ -0,0 +1,66 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { buildWsUrl } from './useAgentEvents';
const SETTINGS_KEY = 'openjarvis-settings';
class MemoryStorage {
private store = new Map<string, string>();
getItem(key: string): string | null {
return this.store.get(key) ?? null;
}
setItem(key: string, value: string): void {
this.store.set(key, String(value));
}
}
beforeEach(() => {
(globalThis as unknown as { localStorage: MemoryStorage }).localStorage =
new MemoryStorage();
});
afterEach(() => {
(globalThis as unknown as { localStorage?: MemoryStorage }).localStorage =
undefined;
});
describe('buildWsUrl', () => {
it('authenticates agent events with the configured API key', () => {
localStorage.setItem(
SETTINGS_KEY,
JSON.stringify({
apiUrl: 'https://jarvis.example.com:8443',
apiKey: 'secret+/=',
}),
);
const url = new URL(buildWsUrl('agent/one'));
expect(url.origin).toBe('wss://jarvis.example.com:8443');
expect(url.pathname).toBe('/v1/agents/events');
expect(url.searchParams.get('agent_id')).toBe('agent/one');
expect(url.searchParams.get('token')).toBe('secret+/=');
});
it('normalizes a versioned API base without duplicating /v1', () => {
localStorage.setItem(
SETTINGS_KEY,
JSON.stringify({ apiUrl: 'http://192.0.2.10:8000/v1/' }),
);
expect(buildWsUrl()).toBe('ws://192.0.2.10:8000/v1/agents/events');
});
it('omits the token for a keyless server', () => {
localStorage.setItem(
SETTINGS_KEY,
JSON.stringify({ apiUrl: 'http://localhost:8000' }),
);
const url = new URL(buildWsUrl('agent-one'));
expect(url.searchParams.has('token')).toBe(false);
});
});
+10 -13
View File
@@ -1,5 +1,5 @@
import { useEffect, useRef } from 'react';
import { getBase } from './api';
import { getApiKey, getBase } from './api';
export interface AgentEvent {
type: string;
@@ -7,19 +7,16 @@ export interface AgentEvent {
data: Record<string, unknown>;
}
function buildWsUrl(agentId?: string): string {
export function buildWsUrl(agentId?: string): string {
const base = getBase();
let origin: string;
if (base) {
origin = base.replace(/^http/, 'ws');
} else {
const loc = window.location;
origin = `${loc.protocol === 'https:' ? 'wss:' : 'ws:'}//${loc.host}`;
}
const path = '/v1/agents/events';
return agentId
? `${origin}${path}?agent_id=${encodeURIComponent(agentId)}`
: `${origin}${path}`;
const url = new URL('/v1/agents/events', base || window.location.origin);
url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:';
if (agentId) url.searchParams.set('agent_id', agentId);
const apiKey = getApiKey();
if (apiKey) url.searchParams.set('token', apiKey);
return url.toString();
}
/**
+2 -1
View File
@@ -127,6 +127,7 @@ class MonitorOperativeAgent(ToolUsingAgent):
memory_backend: Optional[Any] = None,
interactive: bool = False,
confirm_callback=None,
prompt_builder: Optional[Any] = None,
**kwargs: Any,
) -> None:
super().__init__(
@@ -139,7 +140,7 @@ class MonitorOperativeAgent(ToolUsingAgent):
max_tokens=max_tokens,
interactive=interactive,
confirm_callback=confirm_callback,
prompt_builder=kwargs.get("prompt_builder"),
prompt_builder=prompt_builder,
)
# Validate strategies
if memory_extraction not in VALID_MEMORY_EXTRACTION:
+2 -1
View File
@@ -58,6 +58,7 @@ class OperativeAgent(ToolUsingAgent):
memory_backend: Optional[Any] = None,
interactive: bool = False,
confirm_callback=None,
prompt_builder: Optional[Any] = None,
**kwargs: Any,
) -> None:
super().__init__(
@@ -70,7 +71,7 @@ class OperativeAgent(ToolUsingAgent):
max_tokens=max_tokens,
interactive=interactive,
confirm_callback=confirm_callback,
prompt_builder=kwargs.get("prompt_builder"),
prompt_builder=prompt_builder,
)
self._system_prompt = system_prompt or ""
self._operator_id = operator_id
+2 -3
View File
@@ -398,9 +398,8 @@ def _run_agent(
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md persona
# files actually reach the model. Only passed to agents whose __init__
# accepts a `prompt_builder` kwarg (BaseAgent does; agents that override
# __init__ without forwarding it, e.g. OrchestratorAgent, opt out
# automatically and keep their existing system-prompt machinery).
# explicitly accepts a `prompt_builder` kwarg. Agents with specialized
# prompt machinery opt in by naming and forwarding the parameter.
import inspect as _inspect
if "prompt_builder" in _inspect.signature(agent_cls.__init__).parameters:
+21
View File
@@ -367,6 +367,27 @@ def serve(
if getattr(agent_cls, "accepts_tools", False):
agent_kwargs["max_turns"] = config.agent.max_turns
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md
# reach the model on the SERVE path too. ``ask.py`` has done
# this since the persona system landed; ``serve.py`` never did,
# so an agent served over HTTP silently answered as a generic
# assistant while the same agent via the CLI kept its persona.
# Guarded so agents with specialized prompt machinery must opt
# in by explicitly naming and forwarding the kwarg.
import inspect as _inspect
if (
"prompt_builder"
in _inspect.signature(agent_cls.__init__).parameters
):
from openjarvis.prompt.builder import SystemPromptBuilder
agent_kwargs["prompt_builder"] = SystemPromptBuilder(
agent_template=config.agent.default_system_prompt or "",
memory_files_config=config.memory_files,
system_prompt_config=config.system_prompt,
)
agent = agent_cls(engine, model_name, **agent_kwargs)
# Pin MCP transports to the agent's lifetime so HTTP
# connections don't close mid-request (#461).
+14
View File
@@ -516,6 +516,20 @@ class Jarvis:
existing = agent_kwargs.get("tools", [])
agent_kwargs["tools"] = digest_tools + list(existing)
# Wire the SystemPromptBuilder so SOUL.md / MEMORY.md / USER.md reach
# the model — mirrors ``cli/ask.py`` and ``cli/serve.py``. Guarded so
# agents whose ``__init__`` doesn't accept the kwarg opt out.
import inspect as _inspect
if "prompt_builder" in _inspect.signature(agent_cls.__init__).parameters:
from openjarvis.prompt.builder import SystemPromptBuilder
agent_kwargs["prompt_builder"] = SystemPromptBuilder(
agent_template=self._config.agent.default_system_prompt or "",
memory_files_config=self._config.memory_files,
system_prompt_config=self._config.system_prompt,
)
agent_obj = agent_cls(self._engine, model_name, **agent_kwargs)
ctx = AgentContext()
+4 -4
View File
@@ -396,11 +396,10 @@ class TestPersonaFilesReachModel:
assert "MEMORY_SENTINEL" in joined
assert "USER_SENTINEL" in joined
def test_orchestrator_keeps_its_own_system_prompt(
def test_orchestrator_accepts_persona_prompt_builder(
self, runner, monkeypatch, tmp_path
):
"""OrchestratorAgent's __init__ doesn't accept ``prompt_builder``;
the wiring must skip it silently rather than crash."""
"""Orchestrator explicitly accepts and applies persona wiring."""
from openjarvis.core.config import JarvisConfig
soul = tmp_path / "SOUL.md"
@@ -425,5 +424,6 @@ class TestPersonaFilesReachModel:
):
result = runner.invoke(cli, ["ask", "--agent", "orchestrator", "Hello"])
# Pass condition: doesn't crash with TypeError on prompt_builder kwarg.
assert result.exit_code == 0, result.output
messages = engine.generate.call_args.args[0]
assert "ORCH_PERSONA_SENTINEL" in messages[0].content
+129
View File
@@ -0,0 +1,129 @@
"""Regression: ``jarvis serve`` must wire the ``SystemPromptBuilder`` into the
agent it constructs, so SOUL.md / MEMORY.md / USER.md reach the model over HTTP.
``cli/ask.py`` (and ``cli/chat_cmd.py`` and the managed-agent executor) have
wired the builder since the persona system landed. The serve path never did, so
an agent served over HTTP silently answered as a generic assistant explicitly
denying the persona while the same agent via the CLI kept it. Found deploying
a personal assistant: SOUL.md was correct on disk the whole time; no error, no
warning.
This test boots ``serve`` just far enough to capture the agent handed to
``create_app`` and asserts the builder (and thus the persona content) is
present. It fails on the unpatched serve path: ``agent._prompt_builder`` is
``None``, so the persona files never reach the model.
"""
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 a package attribute resolves to the click
# *command* (re-exported); 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"
engine.engine_id = "mock"
return engine
@pytest.mark.parametrize(
"agent_name",
["simple", "orchestrator", "monitor_operative", "operative"],
)
def test_serve_wires_persona_builder_into_served_agent(
tmp_path, monkeypatch, agent_name
):
"""The agent built on the serve path must carry a SystemPromptBuilder whose
assembled prompt includes SOUL.md content (regression for the HTTP persona
loss)."""
from openjarvis.agents.monitor_operative import MonitorOperativeAgent
from openjarvis.agents.operative import OperativeAgent
from openjarvis.agents.orchestrator import OrchestratorAgent
from openjarvis.agents.simple import SimpleAgent
from openjarvis.core.config import JarvisConfig
from openjarvis.core.registry import AgentRegistry
# Persona file with a unique sentinel we can grep for in the built prompt.
soul = tmp_path / "SOUL.md"
soul.write_text("SERVE_PERSONA_SENTINEL", encoding="utf-8")
# conftest clears registries per-test; re-register the agent we exercise.
agent_classes = {
"simple": SimpleAgent,
"orchestrator": OrchestratorAgent,
"monitor_operative": MonitorOperativeAgent,
"operative": OperativeAgent,
}
if not AgentRegistry.contains(agent_name):
AgentRegistry.register_value(agent_name, agent_classes[agent_name])
config = JarvisConfig()
config.server.host = "127.0.0.1"
config.server.port = 8123
config.intelligence.default_model = "test-model"
config.memory_files.soul_path = str(soul)
# Keep the heavy optional subsystems off so we reach create_app cleanly.
config.telemetry.enabled = False
config.agent_manager.enabled = False
config.sessions.enabled = False
config.channel.enabled = False
config.skills.enabled = False
config.agent.context_from_memory = False
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.
sec = MagicMock()
sec.engine = engine
sec.capability_policy = None
sec.audit_logger = None
monkeypatch.setattr("openjarvis.security.setup_security", lambda *a, **k: sec)
captured: dict = {}
def _capture_create_app(*args, **kwargs):
captured["agent"] = kwargs.get("agent")
return MagicMock(name="app")
with (
patch("openjarvis.server.app.create_app", side_effect=_capture_create_app),
patch("uvicorn.run", lambda *a, **k: None),
):
result = CliRunner().invoke(
cli, ["serve", "--agent", agent_name], catch_exceptions=False
)
assert result.exit_code == 0, result.output
agent = captured.get("agent")
assert agent is not None, (
"serve did not construct an agent or never reached create_app; "
f"output:\n{result.output}"
)
# The regression: without the fix ``agent._prompt_builder`` is None and the
# persona files never reach the model over HTTP.
assert agent._prompt_builder is not None, (
f"serve constructed {agent_name} without a prompt_builder — SOUL.md / "
"MEMORY.md / USER.md would be silently dropped on the HTTP path."
)
assert "SERVE_PERSONA_SENTINEL" in agent._prompt_builder.build(), (
"prompt_builder is wired on serve, but its built prompt omits SOUL.md"
)
@@ -0,0 +1,34 @@
"""Regression guards for the desktop app's outbound network policy."""
from __future__ import annotations
import json
import plistlib
from pathlib import Path
ROOT = Path(__file__).resolve().parent.parent.parent
TAURI_CONFIG = ROOT / "frontend" / "src-tauri" / "tauri.conf.json"
MACOS_INFO_PLIST = ROOT / "frontend" / "src-tauri" / "Info.plist"
def _csp_sources(directive: str) -> set[str]:
config = json.loads(TAURI_CONFIG.read_text(encoding="utf-8"))
csp = config["app"]["security"]["csp"]
directives = {
parts[0]: set(parts[1:]) for item in csp.split(";") if (parts := item.split())
}
return directives[directive]
def test_desktop_csp_allows_remote_api_servers() -> None:
"""The user-configured API URL may point beyond localhost (#649)."""
connect_sources = _csp_sources("connect-src")
assert {"http:", "https:", "ws:", "wss:"} <= connect_sources
def test_macos_webview_allows_user_configured_http_servers() -> None:
"""CSP alone cannot override App Transport Security for public hosts."""
info = plistlib.loads(MACOS_INFO_PLIST.read_bytes())
assert info["NSAppTransportSecurity"]["NSAllowsArbitraryLoadsInWebContent"] is True
+24
View File
@@ -95,6 +95,30 @@ class TestJarvisAsk:
assert result == "Agent response"
j.close()
def test_ask_with_agent_wires_persona(self, tmp_path):
from openjarvis.agents.simple import SimpleAgent
from openjarvis.core.registry import AgentRegistry
soul = tmp_path / "SOUL.md"
soul.write_text("SDK_PERSONA_SENTINEL", encoding="utf-8")
cfg = JarvisConfig()
cfg.memory_files.soul_path = str(soul)
cfg.memory_files.memory_path = ""
cfg.memory_files.user_path = ""
cfg.agent.context_from_memory = False
if not AgentRegistry.contains("simple"):
AgentRegistry.register_value("simple", SimpleAgent)
engine = _make_engine()
with patch("openjarvis.sdk.get_engine", return_value=("mock", engine)):
j = Jarvis(config=cfg, model="test-model")
j.ask("Hello", agent="simple")
messages = engine.generate.call_args.args[0]
assert "SDK_PERSONA_SENTINEL" in messages[0].content
j.close()
def test_ask_no_engine_raises(self):
with patch("openjarvis.sdk.get_engine", return_value=None):
j = Jarvis(config=JarvisConfig())