Skip to content

fix(sessions): record session presence regardless of the concurrency cap - #80034

Open
maxmilian wants to merge 1 commit into
NousResearch:mainfrom
maxmilian:fix/46303-registry-always-on
Open

fix(sessions): record session presence regardless of the concurrency cap#80034
maxmilian wants to merge 1 commit into
NousResearch:mainfrom
maxmilian:fix/46303-registry-always-on

Conversation

@maxmilian

@maxmilian maxmilian commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

One decision I'd like from a reviewer before the diff gets read: always-on presence means every session now takes the file lock and writes active_sessions.json, where today most sessions do no I/O here at all — should that stay unconditional (as implemented), or sit behind its own flag, separate from max_concurrent_sessions? It is cheap to change now and considerably more expensive after a review, which is the only reason this sat as a draft. Reasoning expanded under Open question at the bottom.

What does this PR do?

Makes active-session presence tracking work when max_concurrent_sessions isn't set, so "is another session already attached to this checkout?" becomes answerable — the question #46303 opens with.

Today try_acquire_active_session returns before writing anything when the cap is None (hermes_cli/active_sessions.py:285-291), and None is the default (gateway/config.py:920). So for almost every user the registry is permanently empty, and nothing can warn them that a second session is live in the same repo. The reporter's near-clobber was caught by a human reading tmux ls and git status, not by anything the program knew.

This is the second pass @teknium1 invited in the review on #47029"registry-always-on for repo detection, no system-prompt mutation" — and it stays inside the invariants set out there. Both constraints from that review still hold on current main (f5be9236e0); the same cap gate also appears in hermes_cli/status.py:636, which is why hermes status shows nothing today either.

The framing: a None cap should mean "reject nobody", not "know nobody". Presence and enforcement were the same switch; this separates them.

Related Issue

Refs #46303 (addresses the awareness half; the worktree-locking half landed in #48699)

Type of Change

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

Changes Made

  • hermes_cli/active_sessions.py
    • try_acquire_active_session() always records the lease entry; only the rejection path stays conditional on the cap.
    • The whole registry write is best-effort — on any exception it logs and returns the previous no-op lease, so a read-only or corrupt registry can never keep a session from starting.
    • New current_repo_root(): walks up for .git rather than shelling out to git (this runs on every session start). Tests existence rather than is_dir so linked worktrees — precisely the [Bug]: Concurrent sessions cross-contaminate (shared memory injection + shared git worktree) with no isolation or awareness #46303 scenario — are detected.
    • New find_sessions_for_repo(repo_root, *, exclude_lease_id=None).
    • Entries carry metadata["repo_root"] by default, so all three call sites (CLI, gateway, TUI gateway) get attribution without changes; an explicit caller-supplied repo_root still wins.
  • hermes_cli/status.py — drops the if _cap: gate. With a cap it prints slots as before; without one it prints a Live: count. Entries show which checkout they're in, and sessions in the current one are marked ← this repo.
  • tests/hermes_cli/test_active_sessions.py — four tests (below).

Not touched, deliberately, per the #47029 review: self.system_prompt (must stay byte-stable for per-conversation prompt caching) and the Honcho session_strategy default (agreed it deserves its own PR).

How to Test

pytest tests/hermes_cli/test_active_sessions.py -q     # 7 passed

The four new tests, and what each pins:

Test Fails without
test_registry_records_presence_when_cap_is_unset the removal of the max_sessions is None early return
test_uncapped_sessions_are_recorded_but_never_rejected enforcement staying conditional (guards against recording turning into a limit)
test_entries_carry_repo_root_so_sessions_are_attributable the repo_root metadata
test_registry_failure_never_blocks_session_start the best-effort except around the write

I verified those aren't vacuous by reverting each change in turn and confirming only the matching test goes red, then confirming all 7 pass again.

And the new status output, driving that block directly against a scratch HERMES_HOME with two real registry entries — one in this checkout, one elsewhere — rather than a full hermes status run:

  Live:         2 session(s)
                cli               20260806_cli_a1b2        0m  ← this repo
                desktop           20260806_desk_c3d4       0m  other-checkout

With no cap set, that block previously printed nothing at all.

On the "all tests pass" checkbox — I can't honestly tick it, so here is what I actually ran. tests/hermes_cli/ gives 138 failures in my environment, but they are pre-existing: on unmodified main the same suite gives 143. My branch has strictly fewer, and the directly-relevant files (test_active_sessions.py, test_cli_active_session_limit.py, test_status.py, test_session_api.py) are 30/30 green. The noise looks like missing optional dependencies — uv sync can't be used here because uv.lock fails to parse on main (TOML error at line 10, [options]), so I installed with uv pip install -e ".[dev]". Happy to re-run under whatever the canonical setup is.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass — see the note above; pre-existing failures, fewer than on main
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 25.5.0), Python 3.11

Documentation & Housekeeping

  • I've updated relevant documentation — docstrings on both new functions; no user-facing config key changed
  • N/A — no config keys added or changed (max_concurrent_sessions keeps its meaning; it just no longer gates presence)
  • N/A — no architecture or workflow change
  • Cross-platform: current_repo_root() is pure pathlib with no shell-outs; .git-as-file is handled, which is the Windows/worktree case
  • N/A — no tool behavior changed

Open question

Always-on means every session now takes the file lock and writes active_sessions.json, where most sessions previously did no I/O here at all. I've left it unconditional on the reasoning that presence you have to opt into doesn't help the person who didn't know they needed it — but if you'd rather it sat behind its own flag (separate from the cap), that's a small change and better made before review than after.

Presence tracking was gated on `max_concurrent_sessions`, which defaults to
None -- so `try_acquire_active_session` returned before writing any entry for
almost every user, and the registry was empty exactly when someone wanted to
ask whether another session was already attached to a checkout (NousResearch#46303).

A None cap now means "reject nobody", not "know nobody": the lease entry is
always recorded and only *enforcement* stays conditional on the cap. Entries
carry the enclosing git checkout (via the existing `metadata` field), and
`find_sessions_for_repo()` answers the question the issue opens with.

Writing is strictly best-effort -- a read-only or corrupt registry logs and
falls back to the previous no-op lease, so it can never keep a session from
starting.

`hermes status` drops the same cap gate: it already had the display path and
`active_session_registry_snapshot()`, but showed nothing unless a cap was set.
Sessions in the current checkout are marked, which is the reporter's scenario.

Deliberately out of scope, per the review on NousResearch#47029: no mutation of
`self.system_prompt` (it must stay byte-stable for prompt caching), and no
change to the Honcho `session_strategy` default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state area/sessions Session lifecycle, resume, persistence, history labels Aug 6, 2026
@maxmilian
maxmilian marked this pull request as ready for review August 12, 2026 08:56
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 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.

2 participants