Skip to content

fix(gateway): close a profile session's row in its own profile db - #324

Merged
OmarB97 merged 1 commit into
mainfrom
fix/finalize-session-profile-db-fork-20260802
Aug 2, 2026
Merged

fix(gateway): close a profile session's row in its own profile db#324
OmarB97 merged 1 commit into
mainfrom
fix/finalize-session-profile-db-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

_finalize_session (tui_gateway/server.py) wrote a session's ended_at to the wrong database for profile-scoped sessions. It resolved the store with _get_db() — the process-global SessionDB handle pinned to the gateway's launch home, since hermes_state.DEFAULT_DB_PATH is a module-level constant evaluated at import. Every neighbouring session write in that file already routes through the profile-aware _session_db(session) helper; this one did not. Same defect class as #323's /undo branch, one function away.

A session created with profile: "<other>" keeps its row in <root>/profiles/<other>/state.db (session.resume opens it explicitly at server.py:1619), so the launch handle pointed at the wrong file entirely:

  1. The ws_orphan_reap kills gateway-originated sessions, causing Groundhog Day routing loop NousResearch/hermes-agent#60609 guard was inert. db.get_session(session_id) returned None, so source was "", _is_gateway_owned_source("") was False, and _tui_owns_lifecycle became True for every profile session — including a Telegram/Discord session the desktop is only a viewer of.
  2. The end write was a silent no-op. db.end_session(session_id, "tui_close") ran UPDATE ... WHERE id = ? AND ended_at IS NULL against the launch db and matched 0 rows. The row was closed later as agent_close by agent teardown (run_agent.py) — a reason find_latest_gateway_session_for_peer treats as recoverable (hermes_state.py:2504), so a cleanly-closed profile session stayed stale-routable and lingered as a ghost row in /resume.

Routing the write through _session_db(session) fixes both: it opens <profile_home>/state.db when the session carries a profile_home, borrows the shared _get_db() handle otherwise (so the ordinary single-profile path is byte-for-byte unchanged), and closes the per-profile handle on exit.

Does making the guard live reintroduce NousResearch#60609? No — it extends the protection that guard was written for (git log -p -S _is_gateway_owned_sourcef5ef7ee). Checked both consequences for a gateway-owned profile session:

  • DB outcome is identical. The row still ends as the recoverable agent_close from agent teardown — previously by accident of the 0-row UPDATE, now because the guard actually fires. Nothing that was reachable becomes unreachable.
  • One real behavioural change: the Desktop async delegation completions can revive an old session while follow-up prompts create a new session NousResearch/hermes-agent#55578 delegation interrupt. _tui_owns_lifecycle is now False for gateway-owned profile sessions, so closing a viewer tab no longer interrupts the gateway's in-flight background subagents by durable session_key (the tab's own dispatches are still interrupted by origin_ui_session_id). That is exactly what the comment at that call site says should happen — the non-profile path has always behaved this way.

TUI/desktop-owned profile sessions are unaffected by that branch; they simply stop leaving ghost rows.

Related Issue

Found while tracing dead desktop spawns; #321 documented it as deliberately out of scope there. No separate issue filed.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.py_finalize_session acquires the store via with _session_db(session) instead of _get_db(), so the _is_gateway_owned_source lookup and the end_session write both hit the session's own profile db. The ws_orphan_reap kills gateway-originated sessions, causing Groundhog Day routing loop NousResearch/hermes-agent#60609 comment is preserved; a new comment records why the launch handle was wrong.
  • tests/tui_gateway/test_finalize_session_profile_db.py (new) — five tests against real SessionDB files under a temp HERMES_HOME with two profiles (no db mocks): the profile row is ended with tui_close and the launch row is untouched; a gateway-owned profile row is not ended; a session with no profile binding still ends in the launch db (control); and both halves of the now-live guard on the delegation interrupt. Both dbs seed a row under the same id so the write target is unambiguous.
  • tests/test_tui_gateway_server.pytest_finalize_session_profile_session_ends_in_profile_db, following the "launch_update" not in captured contract the existing test_session_resume_profile_uses_profile_db_cwd / test_session_cwd_set_profile_session_updates_profile_db tests use for the resume and cwd paths. The close path had no such cover.

How to Test

  1. The new tests fail on the parent commit and pass here. Reverting only tui_gateway/server.py gives:
    • test_profile_session_is_ended_in_its_own_profile_dbassert row["ended_at"] is not Noneassert None is not None
    • test_gateway_owned_profile_session_is_not_ended → the launch row moved: assert 1785685065.009598 is None
    • test_gateway_owned_profile_session_keeps_gateway_delegationsassert 'sess-profile-1' == ''
    • test_finalize_session_profile_session_ends_in_profile_dbKeyError: 'profile_lookup'
  2. Live check on the real RPC surface — a fresh temp HERMES_HOME with two profiles, driving session.create(profile="worker") → first-message row persist → session.close, then reading both state.db files with plain sqlite3 (see log below).
  3. Suite: scripts/run_tests.sh tests/tui_gateway/ tests/test_tui_gateway_server.py tests/test_lazy_session_regressions.py -q → 45 files, 888 passed, 0 failed. Plus tests/test_tui_gateway_ws.py tests/test_hermes_state.py tests/tools/test_async_delegation.py tests/hermes_cli/test_resolve_last_session.py tests/hermes_state/test_resolve_resume_session_id.py tests/gateway/test_webhook_session_close.py → 446 passed, 0 failed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — ran the affected areas only (1,334 tests across the gateway/session-lifecycle files listed above, 0 failed); the full suite is not green on this machine independent of this change
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.6.0)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Same script, same steps, real sqlite3 read of both dbs after session.close returned {'closed': True}.

Before (parent commit) — the profile row never closes:

PROFILE .../profiles/worker/state.db
          {'id': '20260802_084302_18ce28', 'source': 'desktop',  'ended_at': None, 'end_reason': None}
          {'id': '20260802_084304_4fc0e0', 'source': 'telegram', 'ended_at': None, 'end_reason': None}
LAUNCH  .../state.db
          (no rows)          <- the UPDATE went here and matched nothing

  [FAIL] desktop row is ended there
  [FAIL] ...with end_reason=tui_close

After:

PROFILE .../profiles/worker/state.db
          {'id': '20260802_084247_8f9daf', 'source': 'desktop',  'ended_at': 1785685368.794548, 'end_reason': 'tui_close'}
          {'id': '20260802_084247_2eab38', 'source': 'telegram', 'ended_at': None, 'end_reason': None}
LAUNCH  .../state.db
          (no rows)

  [PASS] desktop row lives in the profile db
  [PASS] desktop row is ended there
  [PASS] ...with end_reason=tui_close
  [PASS] gateway-owned row is NOT ended
  [PASS] launch db holds neither session's row

The telegram row staying open is correct in both runs — but for different reasons, which is the point of the guard: before, because the write missed the file; after, because _is_gateway_owned_source finally reads a real source.

🤖 Generated with Claude Code

`_finalize_session` resolved the session store with `_get_db()`, the
process-global SessionDB handle pinned to the gateway's launch home
(`hermes_state.DEFAULT_DB_PATH` is a module-level constant evaluated at
import). Every neighbouring write in the file already routes through the
profile-aware `_session_db(session)`; this one did not. Same defect class
as #323's /undo branch, one function away.

A session created with `profile: "<other>"` keeps its row in
`<root>/profiles/<other>/state.db`, so the launch handle was the wrong
file, with two consequences:

1. `db.get_session(session_id)` returned None, so `source` was "" and
   `_is_gateway_owned_source("")` was False — `_tui_owns_lifecycle`
   became True for every profile session, including a Telegram/Discord
   one the desktop is only viewing. The NousResearch#60609 Groundhog Day guard was
   INERT for profile sessions.
2. `db.end_session(session_id, "tui_close")` ran
   `UPDATE ... WHERE id = ? AND ended_at IS NULL` against the launch db
   and matched 0 rows — a silent no-op. The row was closed later as
   `agent_close` by agent teardown (run_agent.py), a reason
   `find_latest_gateway_session_for_peer` treats as *recoverable*, so a
   cleanly-closed session stayed stale-routable.

Route the write through `_session_db(session)`: `<profile_home>/state.db`
when the session carries a `profile_home`, the shared `_get_db()` handle
otherwise, so the ordinary single-profile path is unchanged and the
context manager closes the per-profile handle on exit.

Making the guard live does not reintroduce NousResearch#60609 — it extends the
protection it was written for. For a gateway-owned profile session the
db outcome is identical (the row still ends as the recoverable
`agent_close`, previously by accident of the 0-row UPDATE, now because
the guard fires). The one behavioural change there is the NousResearch#55578
delegation interrupt: `_tui_owns_lifecycle` is now False, so closing a
viewer tab no longer interrupts the gateway's background subagents by
durable session_key — exactly what the comment at that call site says
should happen. TUI/desktop-owned profile sessions are unaffected by that
branch and simply stop leaving ghost rows in /resume.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 8646657 into main Aug 2, 2026
32 checks passed
OmarB97 pushed a commit that referenced this pull request Aug 2, 2026
`_get_db()` is a cached module global bound to `hermes_state.DEFAULT_DB_PATH`,
evaluated at *import* time from the launch profile's home. A session created
with `profile: "<other>"` keeps its rows in `<root>/profiles/<other>/state.db`,
reached through the profile-aware `_session_db(session)` helper.

#323 (/undo) and #324 (`_finalize_session`) each fixed one site of this. This
audits every remaining `_get_db()` call in `tui_gateway/server.py` that runs
with a live `session` dict in hand and fixes the twelve that are wrong.

Reproduced against real SessionDB files under a temp home with two profiles:

- `session.branch` died outright with "branch failed: FOREIGN KEY constraint
  failed" — the branch row is an FK child of the parent's row, which the launch
  db does not have. The child now inherits the parent's profile end to end (row,
  agent handle, `profile_home`, and the `_profile_home_bound` binding around
  `_resolve_model()` that #325 established), because fixing only the write would
  have left its rows in one db and its live session pointing at another.
- `prompt.submit`'s edit truncation raised the same FK error, which also skipped
  `deactivate_turn_outcomes_from_ordinal` in the same `try`, and left the profile
  db holding the full pre-edit history — so resuming brought the edited-away
  turns back. Same class as #323.
- Both notification-ownership checks resolved the compression chain in the wrong
  db, so a post-compression profile session stopped recognising its own
  pre-compression dispatches and the fail-closed gate of NousResearch#55578 dropped the
  delegation completion. Both now share `_resolve_session_lineage_key`.
- `/history` replaced the live window with an empty launch-db read and answered
  "No conversation history yet." mid-conversation; `/context` under-reported.
- Titles: the read paths, the post-turn `pending_title` apply, and
  `maybe_auto_title` all used the launch db, so a profile session's title was
  either invisible or never persisted at all.
- `/status` found no row, so it had no Title and reported Created/Last Activity
  as "now"; `_background_agent_kwargs` wrote a profile session's background
  transcript into the launcher's state.db.

Where the handle must outlive the call — `maybe_auto_title`'s daemon thread,
`_background_agent_kwargs`' background agent, and the notification poll loop —
the fix uses the agent's own long-lived handle rather than a `_session_db()` one
that would be closed on block exit (or churn schema write locks per poll),
matching `_persist_live_session_runtime`.

Left launch-scoped deliberately: `session.list`, `session.most_recent`,
`session.delete`, `projects.*` and `insights.get` hold no session and take no
profile — the desktop routes them per-profile by connecting to that profile's
gateway (apps/desktop/src/store/projects.ts:252).

Verified with 20 tests over real SessionDB files under a temp HERMES_HOME with
two profiles, no db mocks; 16 fail on the parent commit and 4 are controls that
pass on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
… db (#333)

`_get_db()` is a cached module global bound to `hermes_state.DEFAULT_DB_PATH`,
evaluated at *import* time from the launch profile's home. A session created
with `profile: "<other>"` keeps its rows in `<root>/profiles/<other>/state.db`,
reached through the profile-aware `_session_db(session)` helper.

#323 (/undo) and #324 (`_finalize_session`) each fixed one site of this. This
audits every remaining `_get_db()` call in `tui_gateway/server.py` that runs
with a live `session` dict in hand and fixes the twelve that are wrong.

Reproduced against real SessionDB files under a temp home with two profiles:

- `session.branch` died outright with "branch failed: FOREIGN KEY constraint
  failed" — the branch row is an FK child of the parent's row, which the launch
  db does not have. The child now inherits the parent's profile end to end (row,
  agent handle, `profile_home`, and the `_profile_home_bound` binding around
  `_resolve_model()` that #325 established), because fixing only the write would
  have left its rows in one db and its live session pointing at another.
- `prompt.submit`'s edit truncation raised the same FK error, which also skipped
  `deactivate_turn_outcomes_from_ordinal` in the same `try`, and left the profile
  db holding the full pre-edit history — so resuming brought the edited-away
  turns back. Same class as #323.
- Both notification-ownership checks resolved the compression chain in the wrong
  db, so a post-compression profile session stopped recognising its own
  pre-compression dispatches and the fail-closed gate of NousResearch#55578 dropped the
  delegation completion. Both now share `_resolve_session_lineage_key`.
- `/history` replaced the live window with an empty launch-db read and answered
  "No conversation history yet." mid-conversation; `/context` under-reported.
- Titles: the read paths, the post-turn `pending_title` apply, and
  `maybe_auto_title` all used the launch db, so a profile session's title was
  either invisible or never persisted at all.
- `/status` found no row, so it had no Title and reported Created/Last Activity
  as "now"; `_background_agent_kwargs` wrote a profile session's background
  transcript into the launcher's state.db.

Where the handle must outlive the call — `maybe_auto_title`'s daemon thread,
`_background_agent_kwargs`' background agent, and the notification poll loop —
the fix uses the agent's own long-lived handle rather than a `_session_db()` one
that would be closed on block exit (or churn schema write locks per poll),
matching `_persist_live_session_runtime`.

Left launch-scoped deliberately: `session.list`, `session.most_recent`,
`session.delete`, `projects.*` and `insights.get` hold no session and take no
profile — the desktop routes them per-profile by connecting to that profile's
gateway (apps/desktop/src/store/projects.ts:252).

Verified with 20 tests over real SessionDB files under a temp HERMES_HOME with
two profiles, no db mocks; 16 fail on the parent commit and 4 are controls that
pass on both.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant