Skip to content

fix(tui-gateway): claim active-session leases lazily and release them when idle (#57052) - #57059

Open
lEWFkRAD wants to merge 2 commits into
NousResearch:mainfrom
lEWFkRAD:fix/tui-idle-session-leases
Open

fix(tui-gateway): claim active-session leases lazily and release them when idle (#57052)#57059
lEWFkRAD wants to merge 2 commits into
NousResearch:mainfrom
lEWFkRAD:fix/tui-idle-session-leases

Conversation

@lEWFkRAD

@lEWFkRAD lEWFkRAD commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops idle desktop/TUI tabs from permanently pinning max_concurrent_sessions slots. Today the TUI gateway claims a cap slot for every open tab (at session.create/resume/branch — i.e. at composer-paint time, before any user input) and only releases it in _finalize_session (tab close / WS-orphan reap / process exit). The idle reaper requires a dead transport, so a healthy connected desktop app never releases anything: N idle overnight tabs sit at N/N and every new session on every surface gets "Hermes is at the active session limit".

The claim was also more eager than the gateway's own persistence policy — the DB row is deliberately created lazily on the first prompt (the in-code NOTE: avoids ghost "Untitled" sessions for every composer paint), yet the lease was claimed at paint time.

This PR fixes the claim/release timing without changing what the cap means ("recently active surfaces"):

  • Lazy claim on the first turn (_ensure_turn_lease, at the top of the _run_prompt_submit run body — the chokepoint every turn entry funnels through: prompt.submit, queued-prompt drain, goal continuation, notification turns). Opening tabs / switching sessions no longer consumes a slot.
  • Idle release via the existing reaper scan (_release_idle_session_leases): a session with no conversational activity for 30 minutes hands back the lease only — the session, agent, and transcript stay live. Skips anything mid-turn, awaiting an input/approval prompt, holding a queued prompt, or still building its agent. HERMES_TUI_LEASE_IDLE_S overrides the window (0 = hold until tab close).
  • Transparent re-acquire on the next turn. If the cap is genuinely full at that moment, the turn surfaces the standard limit message as its error event — the same message.starterror shape as the existing context-injection-refused path, which both the Ink TUI and the desktop client already render as a failed turn.

Why this approach (and not per-turn claim/release like the messaging gateway): releasing after every turn opens a window where a chained turn's re-claim loses to a concurrent surface — and the queued user prompt or notification event dispatched into that turn would be dropped. Holding across turns with an idle-window release frees idle tabs while chained turns (queued prompts, goal continuations) can never lose their slot mid-run. It also keeps the compression re-anchor (_transfer_active_session_slot, incl. the #49041 reserve-before-release fallback) working unchanged on the same session slot.

Races covered: the claim's store is re-checked under history_lock (the same lock every turn-start path sets running under); a lease claimed for a tab that finalized in the claim window is handed straight back; the idle sweep checks running/pending/queued state under that same lock so it can't pull a lease out from under a starting turn.

Related Issue

Fixes #57052

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tui_gateway/server.py
    • New _ensure_turn_lease(sid, session) — claim-or-reuse at turn start, with lost-store-race and finalized-session guards; fail-open behavior preserved.
    • New _release_idle_session_leases(now) + _LEASE_IDLE_RELEASE_S (default 1800s, HERMES_TUI_LEASE_IDLE_S override, mirrors the existing HERMES_TUI_SESSION_TTL_S pattern); wired into _reap_idle_sessions.
    • Turn runner (_run_prompt_submit run body): claim before any model work; at-cap emits the standard limit message as the turn's error.
    • Removed the eager claims + 4090 rejections from session.create, all three session.resume branches, and session.branch (and their now-dead lease error-release paths).
  • hermes_cli/active_sessions.py — module docstring updated to describe the per-surface lease semantics (no code change).
  • website/docs/user-guide/configuration.md — documents what "active" means per surface and the idle window / env override.
  • cli-config.yaml.example — same note on the max_concurrent_sessions key.
  • tests/tui_gateway/test_protocol.py — new coverage: lazy claim + reuse; at-cap limit message; no store into a finalized session; idle release + transparent re-acquire; idle-release exemptions (running / recent activity / queued prompt / pending gateway prompt); HERMES_TUI_LEASE_IDLE_S=0 disable.
  • tests/test_tui_gateway_server.pytest_session_create_rejects_at_active_session_limit reworked to pin the new contract (create claims nothing; the first turn hits the cap; close returns the slot).

How to Test

  1. Set max_concurrent_sessions: 2 in ~/.hermes/config.yaml, start the desktop app (or hermes --tui gateway).
  2. Open two chat tabs without typing, then open a third tab / start a CLI chat: previously rejected with "active session limit (2/2)", now all succeed — HERMES_HOME/runtime/active_sessions.json stays empty until a tab actually sends a message.
  3. Send a message in two tabs (both slots claimed), then try a third surface: it gets the limit message — the cap still enforces concurrent activity.
  4. Let the two tabs sit idle past the window (for a quick check: HERMES_TUI_LEASE_IDLE_S=30, reaper scans every 300s — or call _release_idle_session_leases directly as the tests do): slots free without closing the tabs; typing in a tab again re-acquires transparently.
  5. Automated: pytest tests/tui_gateway/test_protocol.py tests/hermes_cli/test_active_sessions.py tests/gateway/test_max_concurrent_sessions.py tests/hermes_cli/test_cli_active_session_limit.py -q (96 passed) and pytest tests/test_tui_gateway_server.py -q (303 passed).

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 — full-suite run in progress on Windows 11; all lease-related suites green (96 + 303, see How to Test). Will check this box with the result.
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 (native)

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 — N/A (no architecture/workflow change)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no new OS-touching code; lease registry file locking is the existing active_sessions.py implementation (msvcrt/fcntl), unchanged
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Registry during the new lifecycle (cap=1, HERMES_HOME/runtime/active_sessions.json):

open tab (no message yet)  → {"entries": []}                                  # was: 1 slot held
first message              → {"entries": [{"surface": "tui", "session_id": …}]}
30 min idle                → {"entries": []}                                  # tab still open & connected
next message               → re-claims transparently; at cap → turn error:
                             "Hermes is at the active session limit (1/1). Try again when another session finishes."

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 2, 2026
… when idle

The TUI/desktop gateway claimed a max_concurrent_sessions slot for every
OPEN tab (session.create/resume/branch) and only released it in
_finalize_session -- tab close, WS-orphan reap, or process exit. The idle
reaper requires a DEAD transport, so tabs sitting open in a connected
desktop app held their slot forever: 7 idle overnight tabs pin the cap at
7/7 and every new session on any surface gets "Hermes is at the active
session limit". The claim was also more eager than the gateway's own
persistence policy: the DB row is deliberately created lazily on the first
prompt (to avoid ghost "Untitled" sessions for every composer paint), yet
the lease was claimed at paint time.

Fix the release path instead of changing cap semantics:

- Claim lazily on a tab's first turn (_ensure_turn_lease at the top of the
  _run_prompt_submit run body -- the chokepoint every turn entry funnels
  through: prompt.submit, queued drain, goal continuation, notification
  turns). Opening tabs and switching sessions never consumes a slot.
- Keep the lease across turns (no per-turn churn, chained turns can't lose
  their slot mid-run), but hand it back after 30 idle minutes via the
  existing reaper scan (_release_idle_session_leases) -- the lease only;
  the session, agent, and transcript stay live. HERMES_TUI_LEASE_IDLE_S
  overrides the window; 0 keeps slots held until the tab closes.
- The next turn transparently re-acquires. If the cap is full at that
  point, the turn surfaces the standard limit message as its error event
  (same message.start->error shape as the context-injection-refused path)
  and the session returns to idle.
- The store is guarded against a tab finalizing between claim and store
  (the fresh lease is handed straight back) and against concurrent
  claimers (re-checked under history_lock, the same lock every turn-start
  path uses).
- Compression re-anchor (_transfer_active_session_slot) is unchanged: a
  mid-turn lease sits in the same session slot and transfers as before;
  an idle session without a lease re-claims under the new session key on
  its next turn.

The messaging gateway already claims per in-flight turn and the CLI keeps
its process-lifetime lease; docs now spell out the per-surface semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lEWFkRAD
lEWFkRAD force-pushed the fix/tui-idle-session-leases branch from 6d9deac to 8014ea1 Compare July 13, 2026 19:36

@teknium1 teknium1 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.

Thanks for the focused lifecycle fix. The premise reproduces on current main: session.create claims the lease before a user prompt (tui_gateway/server.py:5261), while the idle reaper requires a dead transport (tui_gateway/server.py:860-872). The lazy first-turn claim and idle-release tests are well targeted.

Problems

  • The new user-facing HERMES_TUI_LEASE_IDLE_S setting (tui_gateway/server.py:930, documented at website/docs/user-guide/configuration.md:1646-1648) is a non-secret behavioral configuration knob. AGENTS.md:102-106 requires these settings to live in config.yaml, not a new HERMES_* environment variable.

Suggested changes

  • Add the idle-release duration through DEFAULT_CONFIG, resolve it from config in the TUI gateway, and update the example/docs and tests for the default and 0 disabled behavior.

Automated hermes-sweeper review.

Comment thread website/docs/user-guide/configuration.md Outdated
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 15, 2026
@darshthakkar

Copy link
Copy Markdown

I opened a focused stacked follow-up for the idle-release boundary: lEWFkRAD#2

It replaces sweep-only release with cancellable per-session timers at the actual full-turn idle boundary, preserves warm runtime/history, protects running/queued/pending/building work, and keeps the sweep as a recovery backstop. The branch passes 420 gateway tests plus Ruff/compile checks.

One important rebase note: current main has the process-isolated turn path (_submit_prompt_to_compute_host), which bypasses _run_prompt_submit and therefore bypasses the lazy _ensure_turn_lease claim introduced here. On rebase, that path should claim before compute-host dispatch; at cap it should clear running/inflight state, emit the normal message.start + limit error, drain any queued prompt, and skip model work. I have red/green regressions for that integrated against current main and can publish the focused rebase follow-up once this branch includes the isolation path.

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

[Bug]: Idle desktop/TUI tabs permanently pin max_concurrent_sessions slots (claimed at composer paint, never released while connected)

4 participants