diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b692fc4..2ef35594 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/api/models.py b/api/models.py index 3c8d895d..0a734bc5 100644 --- a/api/models.py +++ b/api/models.py @@ -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, diff --git a/tests/test_default_personality.py b/tests/test_default_personality.py index 28169a0a..98d4c920 100644 --- a/tests/test_default_personality.py +++ b/tests/test_default_personality.py @@ -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" \ No newline at end of file + try: + assert s.personality is None + finally: + with m.LOCK: + m.SESSIONS.pop(s.session_id, None) diff --git a/tests/test_issue798.py b/tests/test_issue798.py index 5f5fd6a1..7957570e 100644 --- a/tests/test_issue798.py +++ b/tests/test_issue798.py @@ -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