mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-31 03:12:16 +00:00
The OpenAI-compatible POST /v1/chat/completions endpoint — the desktop UI's
chat backend — never injected OpenJarvis's agent.default_system_prompt when the
client omits a system message. The frontend (Chat/InputArea.tsx) posts only
user/assistant turns, so the model answered from its training identity
("I'm Claude", "I am Qwen", ...). The CLI paths ground identity via
SystemPromptBuilder / BaseAgent; the engine-direct server handlers did not.
Fix:
- Add _ensure_identity_prompt(messages, app_config) in server/routes.py: returns
messages unchanged when any has role==SYSTEM, else prepends a SYSTEM message
with the resolved identity prompt (app.state.config.agent.default_system_prompt,
else load_config()), wrapped in try/except that debug-logs on failure (no crash,
no silent swallow per REVIEW.md).
- Apply it after _to_messages() in all three engine-direct handlers:
_handle_stream, _handle_stream_tools, and _handle_direct; thread app.state.config
through. _handle_agent is left untouched (BaseAgent already injects the default).
- Harden AgentConfig.default_system_prompt so distilled models stop claiming to be
Claude/ChatGPT/Gemini and self-identify as OpenJarvis.
Tests (tests/server/test_routes.py, tests/core/test_config.py): identity prompt IS
prepended when no system message is present (stream / direct / tools paths) and is
NOT duplicated when the client supplies one; config wording anchors "OpenJarvis"
and "not Claude". Verified fail-on-unfixed against main (3 inject tests + config
wording test fail there).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4b9948250b
commit
28e75cb513
@@ -946,8 +946,10 @@ class AgentConfig:
|
||||
system_prompt_path: str = "" # path to system prompt file (.txt, .md)
|
||||
context_from_memory: bool = True # inject relevant memory context into prompts
|
||||
default_system_prompt: str = (
|
||||
"You are a helpful AI assistant running locally on the user's own "
|
||||
"hardware through OpenJarvis. You are not a cloud service. Respond "
|
||||
"You are OpenJarvis, a helpful AI assistant running locally on the "
|
||||
"user's own hardware. You are not a cloud service, and you are not "
|
||||
"Claude, ChatGPT, Gemini, or any other branded assistant. If asked "
|
||||
"who or what you are, identify yourself as OpenJarvis. Respond "
|
||||
"helpfully, concisely, and accurately."
|
||||
)
|
||||
|
||||
|
||||
@@ -43,6 +43,51 @@ def _to_messages(chat_messages) -> list[Message]:
|
||||
return messages
|
||||
|
||||
|
||||
def _ensure_identity_prompt(messages: list[Message], app_config) -> list[Message]:
|
||||
"""Prepend OpenJarvis's identity system prompt when the client omits one.
|
||||
|
||||
The desktop UI's chat backend posts only user/assistant turns to
|
||||
``/v1/chat/completions`` (see ``frontend/.../Chat/InputArea.tsx``), so
|
||||
nothing grounds the model's identity. Without a system prompt the model
|
||||
answers from its training identity (e.g. "I'm Claude", "I am Qwen"),
|
||||
which is what #540 reported. The CLI paths inject this via
|
||||
``SystemPromptBuilder`` / ``BaseAgent``; the engine-direct server paths
|
||||
did not. This mirrors the agent fallback in ``agents/_stubs.py``.
|
||||
|
||||
If any message already carries a system role, the caller has supplied
|
||||
their own grounding and we leave the list untouched (no double-prompting).
|
||||
|
||||
Resolution of the identity text: ``app_config.agent.default_system_prompt``
|
||||
when a config is wired onto ``app.state``; otherwise fall back to
|
||||
``load_config()``. Config resolution is wrapped so a broken/missing
|
||||
config degrades to "no injection" rather than crashing the endpoint, but
|
||||
the failure is logged (per REVIEW.md — never silently swallow).
|
||||
"""
|
||||
if any(m.role == Role.SYSTEM for m in messages):
|
||||
return messages
|
||||
|
||||
prompt = ""
|
||||
try:
|
||||
if app_config is not None:
|
||||
prompt = app_config.agent.default_system_prompt or ""
|
||||
else:
|
||||
from openjarvis.core.config import load_config
|
||||
|
||||
prompt = load_config().agent.default_system_prompt or ""
|
||||
except Exception:
|
||||
logging.getLogger("openjarvis.server").debug(
|
||||
"Identity system prompt resolution failed; "
|
||||
"serving request without identity grounding",
|
||||
exc_info=True,
|
||||
)
|
||||
return messages
|
||||
|
||||
if not prompt:
|
||||
return messages
|
||||
|
||||
return [Message(role=Role.SYSTEM, content=prompt), *messages]
|
||||
|
||||
|
||||
@router.post("/v1/chat/completions")
|
||||
async def chat_completions(request_body: ChatCompletionRequest, request: Request):
|
||||
"""Handle chat completion requests (streaming and non-streaming)."""
|
||||
@@ -149,7 +194,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
# from the engine for true real-time output.
|
||||
if request_body.tools:
|
||||
return await _handle_stream_tools(
|
||||
engine, model, request_body, complexity_info
|
||||
engine, model, request_body, complexity_info, app_config=config
|
||||
)
|
||||
return await _handle_stream(
|
||||
engine,
|
||||
@@ -157,6 +202,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
request_body,
|
||||
complexity_info,
|
||||
trace_store=getattr(request.app.state, "trace_store", None),
|
||||
app_config=config,
|
||||
)
|
||||
|
||||
# Non-streaming: use agent if available, otherwise direct engine call.
|
||||
@@ -192,6 +238,7 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
|
||||
request_body,
|
||||
bus=bus,
|
||||
complexity_info=complexity_info,
|
||||
app_config=config,
|
||||
)
|
||||
|
||||
|
||||
@@ -201,9 +248,11 @@ def _handle_direct(
|
||||
req: ChatCompletionRequest,
|
||||
bus=None,
|
||||
complexity_info=None,
|
||||
app_config=None,
|
||||
) -> ChatCompletionResponse:
|
||||
"""Direct engine call without agent."""
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
kwargs: dict[str, Any] = {}
|
||||
if req.tools:
|
||||
kwargs["tools"] = req.tools
|
||||
@@ -380,6 +429,8 @@ async def _handle_stream_tools(
|
||||
model: str,
|
||||
req: ChatCompletionRequest,
|
||||
complexity_info=None,
|
||||
*,
|
||||
app_config=None,
|
||||
):
|
||||
"""Stream a raw OpenAI-compat function-calling response via SSE.
|
||||
|
||||
@@ -397,6 +448,7 @@ async def _handle_stream_tools(
|
||||
from openjarvis.server.cloud_router import is_cloud_model
|
||||
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
chunk_id = f"chatcmpl-{uuid.uuid4().hex[:12]}"
|
||||
use_cloud = is_cloud_model(model)
|
||||
|
||||
@@ -491,6 +543,7 @@ async def _handle_stream(
|
||||
complexity_info=None,
|
||||
*,
|
||||
trace_store=None,
|
||||
app_config=None,
|
||||
):
|
||||
"""Stream response using SSE format.
|
||||
|
||||
@@ -509,6 +562,7 @@ async def _handle_stream(
|
||||
)
|
||||
|
||||
messages = _to_messages(req.messages)
|
||||
messages = _ensure_identity_prompt(messages, app_config)
|
||||
chunk_id = f"chatcmpl-{uuid.uuid4().hex[:12]}"
|
||||
|
||||
# Last user message — recorded as the trace query.
|
||||
|
||||
@@ -237,6 +237,14 @@ class TestAgentConfigNew:
|
||||
or isinstance(getattr(ac.__class__, "temperature", None), property) is False
|
||||
)
|
||||
|
||||
def test_default_system_prompt_anchors_identity(self) -> None:
|
||||
"""#540: the hardened wording must name OpenJarvis and explicitly
|
||||
deny the model's training identity so distilled models stop
|
||||
claiming to be Claude/ChatGPT/etc."""
|
||||
prompt = AgentConfig().default_system_prompt
|
||||
assert "OpenJarvis" in prompt
|
||||
assert "not Claude" in prompt
|
||||
|
||||
|
||||
class TestNestedEngineConfig:
|
||||
def test_nested_access(self) -> None:
|
||||
@@ -561,6 +569,7 @@ class TestWhatsAppBaileysChannelConfig:
|
||||
|
||||
def test_mining_config_absent_means_none(tmp_path):
|
||||
from openjarvis.core.config import load_config
|
||||
|
||||
cfg_path = tmp_path / "config.toml"
|
||||
cfg_path.write_text("") # empty config
|
||||
cfg = load_config(cfg_path)
|
||||
|
||||
@@ -487,6 +487,167 @@ class TestChatCompletions:
|
||||
assert data["choices"][0]["finish_reason"] == "stop"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Identity system-prompt injection (#540)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_capturing_engine(captured: list):
|
||||
"""Like ``_make_engine`` but records the messages each path receives.
|
||||
|
||||
``engine.generate`` is a MagicMock so ``call_args`` works on the
|
||||
direct/non-stream path. ``engine.stream`` / ``engine.stream_full`` are
|
||||
plain async-generator FUNCTIONS, so they capture their ``messages``
|
||||
argument into the shared *captured* list from inside the generator body
|
||||
(``call_args`` does not apply to plain functions).
|
||||
"""
|
||||
engine = MagicMock()
|
||||
engine.engine_id = "mock"
|
||||
engine.health.return_value = True
|
||||
engine.list_models.return_value = ["test-model"]
|
||||
engine.generate.return_value = {
|
||||
"content": "ok",
|
||||
"usage": {"prompt_tokens": 5, "completion_tokens": 3, "total_tokens": 8},
|
||||
"model": "test-model",
|
||||
"finish_reason": "stop",
|
||||
}
|
||||
|
||||
async def mock_stream(messages, *, model, temperature=0.7, max_tokens=1024, **kw):
|
||||
captured.append(messages)
|
||||
for token in ["Hello", " ", "world"]:
|
||||
yield token
|
||||
|
||||
async def mock_stream_full(
|
||||
messages, *, model, temperature=0.7, max_tokens=1024, **kw
|
||||
):
|
||||
from openjarvis.engine._stubs import StreamChunk
|
||||
|
||||
captured.append(messages)
|
||||
yield StreamChunk(content="ok", finish_reason="stop")
|
||||
|
||||
engine.stream = mock_stream
|
||||
engine.stream_full = mock_stream_full
|
||||
return engine
|
||||
|
||||
|
||||
class TestIdentityPromptInjection:
|
||||
"""Regression for #540.
|
||||
|
||||
The desktop UI posts only user/assistant turns to the
|
||||
OpenAI-compatible ``/v1/chat/completions`` endpoint, so the engine never
|
||||
saw OpenJarvis's identity system prompt and the model answered from its
|
||||
training identity ("I'm Claude", "I am Qwen", ...). The engine-direct
|
||||
server handlers must now inject ``agent.default_system_prompt`` whenever
|
||||
the client omits a system message — and must NOT inject a second one when
|
||||
the client already supplies their own.
|
||||
"""
|
||||
|
||||
def test_stream_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
# Drain the stream so the generator body runs and records messages.
|
||||
_ = resp.text
|
||||
assert captured, "engine.stream was never called"
|
||||
msgs = captured[-1]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
def test_stream_no_double_injection_when_client_supplies_system(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [
|
||||
{"role": "system", "content": "Be terse."},
|
||||
{"role": "user", "content": "who are you?"},
|
||||
],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
_ = resp.text
|
||||
msgs = captured[-1]
|
||||
system_msgs = [m for m in msgs if m.role.value == "system"]
|
||||
assert len(system_msgs) == 1
|
||||
assert system_msgs[0].content == "Be terse."
|
||||
|
||||
def test_direct_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
# No agent -> non-stream request goes through _handle_direct.
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert engine.generate.called
|
||||
msgs = engine.generate.call_args.args[0]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
def test_direct_no_double_injection_when_client_supplies_system(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [
|
||||
{"role": "system", "content": "Be terse."},
|
||||
{"role": "user", "content": "who are you?"},
|
||||
],
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
msgs = engine.generate.call_args.args[0]
|
||||
system_msgs = [m for m in msgs if m.role.value == "system"]
|
||||
assert len(system_msgs) == 1
|
||||
assert system_msgs[0].content == "Be terse."
|
||||
|
||||
def test_stream_tools_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
client = TestClient(create_app(engine, "test-model"))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
"tools": [{"type": "function", "function": {"name": "calc"}}],
|
||||
"stream": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
_ = resp.text
|
||||
assert captured, "engine.stream_full was never called"
|
||||
msgs = captured[-1]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Models endpoint tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user