mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
Merge pull request #2865 from AJV20/fix/session-personality-default
fix: avoid stamping display personality on sessions (#2845)
This commit is contained in:
nesquena-hermes
@@ -3,6 +3,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- New WebUI sessions no longer persist `display.personality` into per-session `Session.personality`; only explicit personality changes remain durable, preventing stale global display defaults from overriding profile-scoped session behavior. Closes #2845.
|
||||
|
||||
## [v0.51.130] — 2026-05-24 — Release DB (stage-batch12 — 3-PR profile-isolation + boot-precedence + workspace Artifacts tab)
|
||||
|
||||
### Fixed
|
||||
|
||||
+1
-13
@@ -1945,18 +1945,6 @@ def new_session(workspace=None, model=None, profile=None, model_provider=None, p
|
||||
if model_provider:
|
||||
effective_model_provider = model_provider
|
||||
|
||||
# Read default personality from config display.personality
|
||||
_default_personality = None
|
||||
try:
|
||||
from api.config import get_config as _get_cfg_for_personality
|
||||
_cfg_personality = (_get_cfg_for_personality().get('display') or {}).get('personality')
|
||||
if _cfg_personality and isinstance(_cfg_personality, str):
|
||||
_cfg_personality = _cfg_personality.strip().lower()
|
||||
if _cfg_personality and _cfg_personality not in ('default', 'none', 'neutral'):
|
||||
_default_personality = _cfg_personality
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
wt = worktree_info if isinstance(worktree_info, dict) else None
|
||||
workspace_path = (wt.get('path') if wt and wt.get('path') else workspace) if wt else workspace
|
||||
s = Session(
|
||||
@@ -1965,7 +1953,7 @@ def new_session(workspace=None, model=None, profile=None, model_provider=None, p
|
||||
model_provider=effective_model_provider,
|
||||
profile=profile,
|
||||
project_id=project_id,
|
||||
personality=_default_personality,
|
||||
personality=None,
|
||||
worktree_path=wt.get('path') if wt else None,
|
||||
worktree_branch=wt.get('branch') if wt else None,
|
||||
worktree_repo_root=wt.get('repo_root') if wt else None,
|
||||
|
||||
@@ -1,123 +1,48 @@
|
||||
"""Test that new_session() reads display.personality from config and uses it as default.
|
||||
"""Regression coverage for display.personality not becoming durable session state.
|
||||
|
||||
Regression test for the feature that makes /personality taleb sticky across
|
||||
new sessions — when display.personality is set in config.yaml, every new
|
||||
session should inherit it without requiring an explicit /personality command.
|
||||
Issue #2845: display.personality is a display/default hint, but new_session()
|
||||
previously copied it into Session.personality. That made cosmetic config durable
|
||||
per-session state and could override profile-scoped behavior later. Only an
|
||||
explicit /api/personality/set call should persist Session.personality.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# R1: new_session() inherits display.personality from config
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_new_session_reads_default_personality_from_config():
|
||||
"""When display.personality is set to 'taleb', new_session() should
|
||||
create a Session with personality='taleb'."""
|
||||
def test_new_session_does_not_inherit_display_personality_from_config():
|
||||
"""display.personality='taleb' must not stamp Session.personality."""
|
||||
import api.models as m
|
||||
import api.config as cfg_mod
|
||||
|
||||
_cfg = {
|
||||
cfg = {
|
||||
"display": {"personality": "taleb"},
|
||||
"agent": {"personalities": {"taleb": {"system_prompt": "Be like Taleb", "tone": "blunt"}}},
|
||||
}
|
||||
|
||||
with patch.object(cfg_mod, "get_config", return_value=_cfg), \
|
||||
with patch.object(cfg_mod, "get_config", return_value=cfg), \
|
||||
patch.object(m.Session, "save", return_value=None):
|
||||
s = m.new_session(workspace="/tmp/test-personality")
|
||||
|
||||
assert s.personality == "taleb", (
|
||||
f"Expected personality='taleb', got {s.personality!r}"
|
||||
)
|
||||
try:
|
||||
assert s.personality is None
|
||||
finally:
|
||||
with m.LOCK:
|
||||
m.SESSIONS.pop(s.session_id, None)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# R2: 'none', 'default', 'neutral' are treated as no personality
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("personality_value", ["none", "default", "neutral", ""])
|
||||
def test_new_session_ignores_neutral_personality_values(personality_value):
|
||||
"""Values like 'none', 'default', 'neutral', and '' should NOT be set as
|
||||
the session personality — they mean 'no personality overlay'."""
|
||||
|
||||
def test_new_session_still_defaults_to_no_personality_when_config_missing():
|
||||
"""Missing display.personality continues to produce personality=None."""
|
||||
import api.models as m
|
||||
import api.config as cfg_mod
|
||||
|
||||
_cfg = {
|
||||
"display": {"personality": personality_value},
|
||||
"agent": {"personalities": {}},
|
||||
}
|
||||
cfg = {"agent": {"personalities": {}}}
|
||||
|
||||
with patch.object(cfg_mod, "get_config", return_value=_cfg), \
|
||||
patch.object(m.Session, "save", return_value=None):
|
||||
s = m.new_session(workspace="/tmp/test-personality-neutral")
|
||||
|
||||
assert s.personality is None, (
|
||||
f"Expected None for display.personality={personality_value!r}, "
|
||||
f"got {s.personality!r}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# R3: Missing display.personality → personality=None
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_new_session_no_personality_when_config_missing():
|
||||
"""When config has no display.personality (or display section is absent),
|
||||
new_session() should set personality=None."""
|
||||
|
||||
import api.models as m
|
||||
import api.config as cfg_mod
|
||||
|
||||
_cfg = {"agent": {"personalities": {}}} # No display section at all
|
||||
|
||||
with patch.object(cfg_mod, "get_config", return_value=_cfg), \
|
||||
with patch.object(cfg_mod, "get_config", return_value=cfg), \
|
||||
patch.object(m.Session, "save", return_value=None):
|
||||
s = m.new_session(workspace="/tmp/test-personality-missing")
|
||||
|
||||
assert s.personality is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# R4: Config exception is handled gracefully → personality=None
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_new_session_handles_config_exception_gracefully():
|
||||
"""If get_config() raises, we should still get a valid session with
|
||||
personality=None (the try/except should swallow the error)."""
|
||||
|
||||
import api.models as m
|
||||
import api.config as cfg_mod
|
||||
|
||||
def _boom():
|
||||
raise RuntimeError("config exploded")
|
||||
|
||||
with patch.object(cfg_mod, "get_config", side_effect=_boom), \
|
||||
patch.object(m.Session, "save", return_value=None):
|
||||
s = m.new_session(workspace="/tmp/test-personality-boom")
|
||||
|
||||
assert s.personality is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# R5: display.personality is case-insensitive
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_new_session_personality_is_case_insensitive():
|
||||
"""display.personality='Taleb' should be normalized to 'taleb'."""
|
||||
|
||||
import api.models as m
|
||||
import api.config as cfg_mod
|
||||
|
||||
_cfg = {
|
||||
"display": {"personality": "Taleb"},
|
||||
"agent": {"personalities": {"taleb": {"system_prompt": "Be like Taleb"}}},
|
||||
}
|
||||
|
||||
with patch.object(cfg_mod, "get_config", return_value=_cfg), \
|
||||
patch.object(m.Session, "save", return_value=None):
|
||||
s = m.new_session(workspace="/tmp/test-personality-case")
|
||||
|
||||
assert s.personality == "taleb"
|
||||
try:
|
||||
assert s.personality is None
|
||||
finally:
|
||||
with m.LOCK:
|
||||
m.SESSIONS.pop(s.session_id, None)
|
||||
|
||||
@@ -290,6 +290,24 @@ def test_new_session_uses_explicit_profile_default_model_and_provider(tmp_path,
|
||||
m.SESSIONS.pop(s.session_id, None)
|
||||
|
||||
|
||||
def test_new_session_does_not_persist_display_personality(monkeypatch, tmp_path):
|
||||
"""display.personality is a UI/default hint, not durable per-session state."""
|
||||
import api.config as c
|
||||
import api.models as m
|
||||
|
||||
monkeypatch.setattr(c, "get_config", lambda: {"display": {"personality": "kawaii"}})
|
||||
with patch.object(m.Session, 'save', return_value=None):
|
||||
s = m.new_session(workspace=str(tmp_path), profile="default")
|
||||
try:
|
||||
assert s.personality is None, (
|
||||
"new_session() must not stamp display.personality into the persistent "
|
||||
"Session.personality field; only explicit /api/personality/set should."
|
||||
)
|
||||
finally:
|
||||
with m.LOCK:
|
||||
m.SESSIONS.pop(s.session_id, None)
|
||||
|
||||
|
||||
def test_get_hermes_home_for_profile_rejects_path_traversal():
|
||||
"""R19j: get_hermes_home_for_profile() must reject names that don't match
|
||||
_PROFILE_ID_RE (e.g. path traversal like '../../etc') and return the base
|
||||
|
||||
Reference in New Issue
Block a user