Skip to content

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

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-57059
Open

fix(tui-gateway): claim active-session leases lazily and release them when idle (#57052)#853
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-57059

Conversation

@hashbender

Copy link
Copy Markdown
Owner

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 NousResearch#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 NousResearch#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


Mirror-of: NousResearch#57059
NousResearch#57059

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.

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

1 participant