fix(cli,gateway): surface title errors from /new <name>
The contributor's PR silently swallowed ValueError from SessionDB.set_session_title() with bare except Exception: pass. Users typing /new <title> with an already-in-use title got an untitled session and no feedback. Changes: - cli.py: catch ValueError from both sanitize_title() and set_session_title(); print the error and mark the session untitled in the banner (never echo the rejected title back). - gateway/run.py: append a warning note to the reset reply on title rejection; reflect the accepted title in the header. - Add regression tests for the duplicate-title path in CLI and gateway. Also map exx@example.com -> @exxmen in scripts/release.py.
This commit is contained in:
21
cli.py
21
cli.py
@@ -4988,14 +4988,27 @@ class HermesCLI:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
if title and self._session_db:
|
if title and self._session_db:
|
||||||
|
from hermes_state import SessionDB
|
||||||
try:
|
try:
|
||||||
from hermes_state import SessionDB
|
|
||||||
sanitized = SessionDB.sanitize_title(title)
|
sanitized = SessionDB.sanitize_title(title)
|
||||||
if sanitized:
|
except ValueError as e:
|
||||||
|
_cprint(f" Title rejected: {e}")
|
||||||
|
sanitized = None
|
||||||
|
title = None
|
||||||
|
if sanitized:
|
||||||
|
try:
|
||||||
self._session_db.set_session_title(self.session_id, sanitized)
|
self._session_db.set_session_title(self.session_id, sanitized)
|
||||||
self._pending_title = None
|
self._pending_title = None
|
||||||
except Exception:
|
title = sanitized
|
||||||
pass
|
except ValueError as e:
|
||||||
|
_cprint(f" {e} — session started untitled.")
|
||||||
|
title = None
|
||||||
|
except Exception:
|
||||||
|
title = None
|
||||||
|
elif title is not None:
|
||||||
|
# sanitize_title returned empty (whitespace-only / unprintable)
|
||||||
|
_cprint(" Title is empty after cleanup — session started untitled.")
|
||||||
|
title = None
|
||||||
# Notify memory providers that session_id rotated to a fresh
|
# Notify memory providers that session_id rotated to a fresh
|
||||||
# conversation. reset=True signals providers to flush accumulated
|
# conversation. reset=True signals providers to flush accumulated
|
||||||
# per-session state (_session_turns, _turn_counter, _document_id).
|
# per-session state (_session_turns, _turn_counter, _document_id).
|
||||||
|
|||||||
@@ -6898,14 +6898,26 @@ class GatewayRunner:
|
|||||||
|
|
||||||
# Set session title if provided with /new <title>
|
# Set session title if provided with /new <title>
|
||||||
_title_arg = event.get_command_args().strip()
|
_title_arg = event.get_command_args().strip()
|
||||||
|
_title_note = ""
|
||||||
if _title_arg and self._session_db and new_entry:
|
if _title_arg and self._session_db and new_entry:
|
||||||
|
from hermes_state import SessionDB
|
||||||
try:
|
try:
|
||||||
from hermes_state import SessionDB
|
|
||||||
sanitized = SessionDB.sanitize_title(_title_arg)
|
sanitized = SessionDB.sanitize_title(_title_arg)
|
||||||
if sanitized:
|
except ValueError as e:
|
||||||
|
sanitized = None
|
||||||
|
_title_note = f"\n⚠️ Title rejected: {e}"
|
||||||
|
if sanitized:
|
||||||
|
try:
|
||||||
self._session_db.set_session_title(new_entry.session_id, sanitized)
|
self._session_db.set_session_title(new_entry.session_id, sanitized)
|
||||||
except Exception:
|
header = f"✨ New session started: {sanitized}"
|
||||||
pass
|
except ValueError as e:
|
||||||
|
_title_note = f"\n⚠️ {e} — session started untitled."
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
elif not _title_note:
|
||||||
|
# sanitize_title returned empty (whitespace-only / unprintable)
|
||||||
|
_title_note = "\n⚠️ Title is empty after cleanup — session started untitled."
|
||||||
|
header = header + _title_note
|
||||||
|
|
||||||
# Fire plugin on_session_reset hook (new session guaranteed to exist)
|
# Fire plugin on_session_reset hook (new session guaranteed to exist)
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -513,6 +513,7 @@ AUTHOR_MAP = {
|
|||||||
"nftpoetrist@gmail.com": "nftpoetrist", # PR #18982
|
"nftpoetrist@gmail.com": "nftpoetrist", # PR #18982
|
||||||
"millerc79@users.noreply.github.com": "millerc79", # PR #19033
|
"millerc79@users.noreply.github.com": "millerc79", # PR #19033
|
||||||
"hermes@example.com": "shellybotmoyer", # PR #18915 (bot-committed)
|
"hermes@example.com": "shellybotmoyer", # PR #18915 (bot-committed)
|
||||||
|
"exx@example.com": "exxmen", # PR #19555
|
||||||
"hypnosis.mda@gmail.com": "Hypn0sis",
|
"hypnosis.mda@gmail.com": "Hypn0sis",
|
||||||
"ywt000818@gmail.com": "OwenYWT",
|
"ywt000818@gmail.com": "OwenYWT",
|
||||||
"dhandhalyabhavik@gmail.com": "v1k22",
|
"dhandhalyabhavik@gmail.com": "v1k22",
|
||||||
|
|||||||
@@ -238,3 +238,40 @@ def test_new_session_with_title(capsys):
|
|||||||
|
|
||||||
captured = capsys.readouterr()
|
captured = capsys.readouterr()
|
||||||
assert "My Test Session" in captured.out
|
assert "My Test Session" in captured.out
|
||||||
|
|
||||||
|
|
||||||
|
def test_new_session_with_duplicate_title_surfaces_error(capsys):
|
||||||
|
"""new_session(title=...) handles ValueError from a duplicate-title conflict.
|
||||||
|
|
||||||
|
The session is still created; the title assignment fails; the success banner
|
||||||
|
must not claim the rejected title as the session name.
|
||||||
|
"""
|
||||||
|
cli = _make_cli()
|
||||||
|
cli._session_db = MagicMock()
|
||||||
|
cli._session_db.set_session_title.side_effect = ValueError(
|
||||||
|
"Title 'Dup' is already in use by session abc-123"
|
||||||
|
)
|
||||||
|
cli.agent = _FakeAgent("old_session_id", datetime.now())
|
||||||
|
cli.conversation_history = []
|
||||||
|
|
||||||
|
# Capture warnings printed via cli._cprint. After importlib.reload(),
|
||||||
|
# the method's __globals__ dict is the one from the live module — patch
|
||||||
|
# the exact dict the method will read.
|
||||||
|
warnings: list[str] = []
|
||||||
|
method_globals = cli.new_session.__globals__
|
||||||
|
original = method_globals["_cprint"]
|
||||||
|
method_globals["_cprint"] = lambda msg: warnings.append(msg)
|
||||||
|
try:
|
||||||
|
cli.new_session(title="Dup")
|
||||||
|
finally:
|
||||||
|
method_globals["_cprint"] = original
|
||||||
|
|
||||||
|
cli._session_db.set_session_title.assert_called_once()
|
||||||
|
joined = "\n".join(warnings)
|
||||||
|
assert "already in use" in joined
|
||||||
|
assert "session started untitled" in joined
|
||||||
|
|
||||||
|
# The success banner must NOT claim the rejected title as the session name.
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "New session started: Dup" not in captured.out
|
||||||
|
assert "New session started!" in captured.out
|
||||||
|
|||||||
@@ -274,6 +274,71 @@ class TestResetCommandWithTitle:
|
|||||||
runner._session_db.set_session_title.assert_called_once_with(
|
runner._session_db.set_session_title.assert_called_once_with(
|
||||||
"sess-new", "Custom Name"
|
"sess-new", "Custom Name"
|
||||||
)
|
)
|
||||||
|
# Header reflects the applied title
|
||||||
|
assert "Custom Name" in str(result)
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reset_command_duplicate_title_surfaces_warning(self):
|
||||||
|
"""/new <title> with an already-in-use title returns a warning in the reply."""
|
||||||
|
from datetime import datetime
|
||||||
|
|
||||||
|
from gateway.run import GatewayRunner
|
||||||
|
from gateway.session import SessionEntry, SessionSource, build_session_key
|
||||||
|
|
||||||
|
runner = object.__new__(GatewayRunner)
|
||||||
|
runner.config = GatewayConfig(
|
||||||
|
platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="***")}
|
||||||
|
)
|
||||||
|
adapter = MagicMock()
|
||||||
|
adapter.send = AsyncMock()
|
||||||
|
runner.adapters = {Platform.TELEGRAM: adapter}
|
||||||
|
runner._voice_mode = {}
|
||||||
|
runner.hooks = SimpleNamespace(emit=AsyncMock(), loaded_hooks=False)
|
||||||
|
runner._session_model_overrides = {}
|
||||||
|
runner._pending_model_notes = {}
|
||||||
|
runner._background_tasks = set()
|
||||||
|
|
||||||
|
source = SessionSource(
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
user_id="12345",
|
||||||
|
chat_id="67890",
|
||||||
|
user_name="testuser",
|
||||||
|
)
|
||||||
|
session_key = build_session_key(source)
|
||||||
|
new_session_entry = SessionEntry(
|
||||||
|
session_key=session_key,
|
||||||
|
session_id="sess-new",
|
||||||
|
created_at=datetime.now(),
|
||||||
|
updated_at=datetime.now(),
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
chat_type="dm",
|
||||||
|
)
|
||||||
|
runner.session_store = MagicMock()
|
||||||
|
runner.session_store.get_or_create_session.return_value = new_session_entry
|
||||||
|
runner.session_store.reset_session.return_value = new_session_entry
|
||||||
|
runner.session_store._entries = {session_key: new_session_entry}
|
||||||
|
runner.session_store._generate_session_key.return_value = session_key
|
||||||
|
runner._running_agents = {}
|
||||||
|
runner._pending_messages = {}
|
||||||
|
runner._pending_approvals = {}
|
||||||
|
runner._session_db = MagicMock()
|
||||||
|
runner._session_db.set_session_title.side_effect = ValueError(
|
||||||
|
"Title 'Dup' is already in use by session abc-123"
|
||||||
|
)
|
||||||
|
runner._agent_cache = {}
|
||||||
|
runner._agent_cache_lock = None
|
||||||
|
runner._is_user_authorized = lambda _source: True
|
||||||
|
runner._format_session_info = lambda: ""
|
||||||
|
|
||||||
|
event = _make_event(text="/new Dup")
|
||||||
|
result = await runner._handle_reset_command(event)
|
||||||
|
|
||||||
|
runner._session_db.set_session_title.assert_called_once()
|
||||||
|
reply = str(result)
|
||||||
|
assert "already in use" in reply
|
||||||
|
assert "session started untitled" in reply
|
||||||
|
# Header must NOT claim the rejected title as the session name
|
||||||
|
assert "New session started: Dup" not in reply
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user