Skip to content

fix(cli): replace misleading "Starting fresh" message on empty-session resume (#27168) - #27203

Closed
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/cli-empty-resume-message-27168
Closed

fix(cli): replace misleading "Starting fresh" message on empty-session resume (#27168)#27203
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/cli-empty-resume-message-27168

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

Summary

Rewords both --resume paths in cli.py so the message printed when a
session exists in sessions but has zero rows in messages matches
what the code actually does (reuse the empty session) and surfaces the
remediation the reporter wanted (hermes sessions delete <id>).
Behavior is unchanged — only the user-facing message.

Fixes the user-visible piece of #27168 (Bug 1). Bug 2 (the ~5 s
startup hang in _check_compression_model_feasibility) is out of
scope here — the reporter notes they could not localize it, and
chasing it would be speculative without a clean repro.

The bug

Both _init_agent (cli.py:4445) and _preload_resumed_session
(cli.py:4711) printed:

Session <id> found but has no messages. Starting fresh.

The "Starting fresh" claim is inaccurate — the empty session is
reused: its ended_at is cleared and the user's next message
becomes the first one for that session ID. No new session is created.
Users reading the message reasonably expected a brand-new session,
then saw the same orphan ID stick around.

The fix

One-line wording change at each call site. Both now print:

Session <id> has no prior messages — resuming as an empty session. (To remove it instead, run hermes sessions delete <id>.)

  • Accurate ("resuming as an empty session" matches the ended_at = NULL reopen that follows).
  • Actionable (mentions the sessions delete command the reporter wanted as a remediation).
  • No behavior change — same return values, same DB writes, same flow.

Test plan

  • Focused regression test: tests/cli/test_resume_display.py::TestPreloadResumedSession::test_returns_false_when_session_has_no_messages updated to assert the new wording (no prior messages, empty session, hermes sessions delete empty_session) and that the old Starting fresh text is gone. Also fixes a pre-existing gap in the test where resolve_resume_session_id was not mocked, so the mock returned a MagicMock that the production code then mistook for a redirected session ID.
  • Adjacent suite: tests/cli/test_resume_display.py — all 37 tests pass under uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest.
  • Regression guard: the new assertions ("no prior messages" in output, "Starting fresh" not in output, "hermes sessions delete empty_session" in output) all fail against the old wording and pass against the new wording.

Sibling code paths that may need the same fix: both _init_agent and _preload_resumed_session were updated in this PR. No other call site prints this string. If a future maintainer wants stronger behavior (auto-delete the orphan, or refuse the resume entirely rather than reopen the empty session), happy to follow up — those are behavior changes I intentionally left out of this message-only fix.

Related

…n resume (NousResearch#27168)

When `hermes --resume <id>` targets a session that exists in `sessions` but
has zero rows in `messages`, both resume paths printed:

    Session <id> found but has no messages. Starting fresh.

The "Starting fresh" claim is inaccurate — the empty session is reused
(its `ended_at` is cleared and the user's next message becomes the
first), no new session is created. Users reading the message reasonably
expected a brand-new session, then saw the same orphan ID stick around.

Rewords both call sites in `cli.py` to describe what actually happens
("resuming as an empty session") and surface the actionable remediation
the reporter actually wanted ("`hermes sessions delete <id>` to remove it
instead"). Behavior is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 17, 2026 01:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the "empty session resume" messaging to accurately reflect that the existing empty session is reused rather than replaced, and points users to the sessions delete command if they want removal instead.

Changes:

  • Replaced the misleading "Starting fresh." wording in two CLI code paths with clearer messaging.
  • Added a remediation hint pointing to hermes sessions delete <session_id>.
  • Updated the corresponding regression test to assert the new wording.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
cli.py Updates two empty-session resume messages in _init_agent and _preload_resumed_session.
tests/cli/test_resume_display.py Adjusts test assertions to match the new wording and adds a resolve_resume_session_id mock.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cli.py Outdated
Comment on lines +4712 to +4715
f"[{accent_color}]Session {self.session_id} has no prior "
f"messages — resuming as an empty session. "
f"[dim](To remove it instead, run "
f"`hermes sessions delete {self.session_id}`.)[/][/]"
assert "empty session" in output
assert "Starting fresh" not in output
assert "hermes sessions delete empty_session" in output

Comment thread cli.py
Comment on lines +4445 to +4446
f"[bold {_accent_hex()}]Session {_escape(self.session_id)} has no prior messages — resuming as an empty session.[/] "
f"[dim](To remove it instead, run `hermes sessions delete {_escape(self.session_id)}`.)[/]"
@briandevans

Copy link
Copy Markdown
Contributor Author

CI audit — ruff enforcement (blocking) failure is a pre-existing baseline on clean origin/main (3b3909690, e.g. run 25976714254). Zero failures are in touched code (cli.py, tests/cli/test_resume_display.py).

Check Symptom Root cause on main
ruff enforcement (blocking) PLW1514: pathlib.Path(...).read_text without explicit encoding argument at hermes_cli/send_cmd.py:61:20 Untouched file; ruff check hermes_cli/send_cmd.py reports the same error on a clean checkout of origin/main (3b3909690).

Reproduced locally with uv run --with ruff ruff check hermes_cli/send_cmd.py against the upstream version of the file — identical error text.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 17, 2026
…ion test

Addresses Copilot review on NousResearch#27203:
- Apply `_escape(self.session_id)` in `_preload_resumed_session`'s
  empty-session branch so a session id containing Rich markup metacharacters
  (e.g. `[`) cannot misrender or raise MarkupError. Sibling branch in
  `_init_agent` already escaped; this keeps the two paths aligned.
- Add a regression test asserting the `_init_agent` empty-session branch
  surfaces the same NousResearch#27168 wording (`no prior messages` / `empty session` /
  `hermes sessions delete <id>`, never `Starting fresh`), so the two copies
  cannot drift in opposite directions in future edits.

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

Copy link
Copy Markdown
Contributor Author

@copilot All findings addressed in commit 1ef6d06:

  • cli.py:4715 (raw self.session_id in markup) — Now uses safe_session_id = _escape(self.session_id) and interpolates that value twice. Matches the sibling _init_agent branch, so a session id containing [ cannot misrender or raise MarkupError.
  • tests/cli/test_resume_display.py (no test for _init_agent branch) — Added test_init_agent_empty_session_shows_new_wording, which exercises the _init_agent empty-restored branch directly (_resumed=True, empty conversation_history, mock DB returning no messages) and asserts the same regression guards: no prior messages / empty session / hermes sessions delete <id> present, Starting fresh absent. Both copies of the wording are now pinned by tests.
  • cli.py:4446 (duplication suggestion) — Left the two render sites as-is for this PR; the Rich markup styling differs between them ([bold {accent}] vs [{accent}]) for reasons that predate this change, so factoring out a helper would either change rendered appearance or introduce both-styles plumbing that's outside the scope of Bug Report: Session resume produces confusing output + startup hang when compression is unconfigured #27168. The new _init_agent regression test prevents the wording itself from drifting, which was the underlying maintenance concern.

CI audit — both job failures on this PR are pre-existing baselines on clean origin/main (3b3909690):

Job Failing tests Touches code in this PR?
ruff enforcement (blocking) pre-existing on main (see run 25976714254) no
test tests/acp/test_registry_manifest.py::test_agent_json_version_matches_pyproject (0.13.0 vs 0.14.0), tests/hermes_cli/test_startup_plugin_gating.py::test_builtin_set_covers_every_registered_subcommand (missing send in _BUILTIN_SUBCOMMANDS), tests/hermes_cli/test_gateway_service.py::TestSystemUnitHermesHome::* (PermissionError on /root/.hermes — CI-as-root artifact), tests/tools/test_transcription_dotenv_fallback.py::* (xai dotenv fallback) no

Both reproduce on clean origin/main locally; zero failures are in cli.py or tests/cli/test_resume_display.py.

@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to keep the queue clean — branch is several hundred commits behind main and never picked up a review. Happy to reopen if the underlying fix is still useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants