Compare commits

...
Author SHA1 Message Date
03c5ec3e40 fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458) (#497)
* fix(chat): wire SystemPromptBuilder so persona files load in jarvis chat (fixes #458)

* fix(chat): make `--persona none` actually disable persona files

This PR exposes `--persona none`, but SystemPromptBuilder._load_file read
empty paths as "." (Path("") -> ".") and raised IsADirectoryError, so the
documented opt-out crashed. Guard empty path_str so the "none" opt-out
(which _resolve_persona maps to empty file paths) cleanly injects no
persona. Adds an end-to-end regression test (building with persona
"none" must not raise). Also merges current main (branch was stale).

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

---------

Co-authored-by: Jon Saad-Falcon <jonsaadfalcon@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 14:10:13 -07:00
3 changed files with 68 additions and 0 deletions
+43
View File
@@ -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))
+5
View File
@@ -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 ""
+20
View File
@@ -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)