Skip to content

fix(tui): keep in-session agent rebuilds on their own profile store - #104143

Closed
HexLab98 wants to merge 2 commits into
NousResearch:mainfrom
HexLab98:fix/104079-compression-double-write
Closed

HexLab98 wants to merge 2 commits into
NousResearch:mainfrom
HexLab98:fix/104079-compression-double-write

Conversation

@HexLab98

@HexLab98 HexLab98 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Investigating #104079 turned up two things.

The reported active=0 / active=1 duplicate pairs are not a bug. They are the designed signature of in-place compaction: SessionDB.archive_and_compact() soft-archives the pre-compaction rows (active=0, still searchable) and inserts the compacted transcript as fresh active=1 rows, so the protected tail legitimately exists twice in messages. The issue's detection query groups by session_id, role, timestamp, content without filtering active, so it counts every compaction generation as a duplicate. Live reads are unaffected — get_messages() filters active=1, and _dedupe_display_rows collapses generations for display. stamp_db_persisted_markers (#98450) and the in-place commit rollback (#99477) already cover the real double-INSERT class, which produces duplicate active=1 rows instead.

The cross-profile write drift in the issue's evidence #4 is real, and this PR fixes it. Two paths swap a fresh AIAgent into a live session:

  • _sync_bot_capabilities (tui_gateway/model_switch.py) — runs at every turn start for Bot Chat sessions and rebuilds when the capability fingerprint changes.
  • _reset_session_agent (tui_gateway/agent_callbacks.py) — /new and the tools.set RPC.

Both called _make_agent with neither the session's state.db handle nor its HERMES_HOME. _make_agent resolves prompt/skills/toolsets through get_hermes_home() and defaults session_db to the process-wide launch _get_db() handle, which ignores the profile ContextVar override entirely. A named-profile session therefore migrated onto the launch profile mid-conversation: every later turn appended to ~/.hermes/state.db under the same session_id while the desktop replayed profiles/<name>/state.db, which is exactly the reported "96 messages in the default profile's store" and "chat history reverted to an older point". Compression does not cause this — it only raises the flush volume, which is why the symptom clusters around a compaction.

_start_agent_build already gets this right (_bind_build_profile_scopes + fail-closed _open_profile_session_db, added for #88532/#50233); the two rebuild sites were never brought along.

Both now route through _rebuild_session_agent, which binds the session's profile scopes for the build and inherits the outgoing agent's handle — same session, same file, so no second refcount is acquired and ownership moves across so teardown still releases it exactly once. With no live agent to inherit from it opens the profile store the same fail-closed way the deferred build does, rather than letting _make_agent reach for the launch handle.

Test plan

Two invariant tests added to tests/hermes_state/test_named_profile_session_db.py, the existing home of this bug class ("a named-profile session must never touch the launch state.db"), one per rebuild site. They assert the rebuilt agent holds the session's own handle, that get_hermes_home() inside the build is the profile home, that ownership moved off the outgoing agent, and — writing through the rebuilt agent against real SessionDB handles on a temp HERMES_HOME — that the row lands in profiles/worker/state.db with the launch store still empty.

  • Proven red on base: both fail with assert None is <SessionDB ...> (no handle passed → the launch default).
  • scripts/run_tests.sh tests/hermes_state/test_named_profile_session_db.py → 7 passed.
  • scripts/run_tests.sh tests/test_tui_gateway_server.py → 637 passed.
  • scripts/run_tests.sh tests/tui_gateway/ tests/hermes_state/ → the only failures (test_projects_rpc.py, test_hud_surface_note.py, test_live_db_guard_ancestry.py, test_session_cwd_follow.py) reproduce identically on an unmodified tree; they are a local git init sandbox restriction, not related to this change.

_sync_bot_capabilities (Bot Chat capability refresh, run at every turn start) and
_reset_session_agent (/new, tools.set) swap a fresh AIAgent into a LIVE session but
called _make_agent with neither the session's state.db handle nor its HERMES_HOME.
_make_agent resolves prompt/skills/toolsets through get_hermes_home() and defaults
session_db to the process-wide LAUNCH _get_db() handle, so a named-profile session
silently migrated onto the launch profile: every later turn appended to
~/.hermes/state.db under the same session id while the desktop replayed
profiles/<name>/state.db and showed a stale transcript.

Route both rebuilds through _rebuild_session_agent, which binds the session's
profile scopes and inherits the outgoing agent's handle (same session, same file)
so no second refcount is taken and teardown still releases it exactly once.
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Summary

Fixes the same #104079 stale-transcript symptom from the TUI side: _sync_bot_capabilities and _reset_session_agent rebuilt a live session's agent without its state.db handle or HERMES_HOME, silently migrating named-profile sessions onto the launch store. Both sites now go through a shared _rebuild_session_agent helper that inherits the outgoing agent's handle, binds the profile scopes for the build, carries ownership across (exactly-once teardown), and opens the profile store fail-closed when there is no live agent to inherit from.

Findings (all Non-blocking)

  • Failure handling looks right: if _make_agent raises after opening a dedicated handle, it's closed before re-raise; scopes release in finally. Good.
  • One defensive corner: if _transfer_db_to_agent ever refused an opened dedicated handle, the elif opened branch would close a db the new agent was just built with. That outcome shouldn't occur (dedicated handles transfer), but a debug log there would make a future surprise diagnosable.
  • With neither handle nor profile_home, the helper falls back to the launch db — i.e. the old behavior, fail-open. Reasonable.

Verdict

Safe fix with tests proving writes land in the profile store, not the launch store. No blocking issues.

@teknium1

Copy link
Copy Markdown
Collaborator

Already on main as 8da23bd. Thank you.

@teknium1 teknium1 closed this Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants