diff --git a/hermes_cli/active_sessions.py b/hermes_cli/active_sessions.py index 20587125c0ca..4c28de06b555 100644 --- a/hermes_cli/active_sessions.py +++ b/hermes_cli/active_sessions.py @@ -392,6 +392,25 @@ def _optional_float(value: Any) -> Optional[float]: return None +def current_repo_root(start: Optional[Path] = None) -> Optional[str]: + """Return the enclosing git checkout for ``start``, or None if there is none. + + Walks up looking for ``.git`` rather than shelling out to ``git``: this runs + on every session start, and a subprocess per session is a poor trade for a + field that is advisory. ``.git`` is a file (not a directory) inside linked + worktrees, which is why this tests existence rather than is_dir -- sessions + in two worktrees of one repo are exactly the case #46303 is about. + """ + try: + current = (start or Path.cwd()).resolve() + except OSError: + return None + for candidate in (current, *current.parents): + if (candidate / ".git").exists(): + return str(candidate) + return None + + def _pid_liveness(pid: Any, process_start_time: Any = None) -> Optional[bool]: """Return True/False for live/dead, or None when liveness is unknowable.""" try: @@ -504,10 +523,20 @@ def _lease_entry( } if track_liveness: entry["track_liveness"] = True - if metadata: - entry["metadata"] = { - str(k): v for k, v in metadata.items() if isinstance(k, str) - } + entry_metadata = { + str(k): v for k, v in (metadata or {}).items() if isinstance(k, str) + } + # Repo attribution is what makes "which checkout is that session attached + # to?" answerable at all (#46303). Recorded here rather than by caller + # opt-in so every surface gets it without threading it through three + # separate call sites; an explicit metadata repo_root wins, and a session + # started outside any checkout simply records nothing. + if "repo_root" not in entry_metadata: + _repo_root = current_repo_root() + if _repo_root: + entry_metadata["repo_root"] = _repo_root + if entry_metadata: + entry["metadata"] = entry_metadata return entry @@ -735,9 +764,18 @@ def transfer_active_session( entry["session_id"] = new_session_id entry["updated_at"] = time.time() if metadata: - entry["metadata"] = { + new_metadata = { str(k): v for k, v in metadata.items() if isinstance(k, str) } + # Carry the checkout attribution across a session-id transfer: + # callers pass identity metadata, not a full replacement, and + # the process has not changed repos (#46303). + prior = entry.get("metadata") + if "repo_root" not in new_metadata and isinstance(prior, dict): + prior_root = prior.get("repo_root") + if prior_root: + new_metadata["repo_root"] = prior_root + entry["metadata"] = new_metadata updated = True break if not updated and lease.track_liveness: diff --git a/hermes_cli/status.py b/hermes_cli/status.py index f5c435d91267..8a89840ded78 100644 --- a/hermes_cli/status.py +++ b/hermes_cli/status.py @@ -663,25 +663,32 @@ def _resolve_env(env_ref) -> str: else: print(f" Active: {_session_count if _session_count is not None else 0}") - # Slot usage, only when max_concurrent_sessions is set. The cap is shared - # across CLI, desktop/TUI and the messaging gateway, so the surface that - # gets rejected is rarely the one holding the slots — without this the only - # way to find out is reading runtime/active_sessions.json by hand. + # Live sessions from the active-session registry. Slot usage is only + # meaningful when max_concurrent_sessions is set — the cap is shared across + # CLI, desktop/TUI and the messaging gateway, so the surface that gets + # rejected is rarely the one holding the slots — but the sessions + # themselves are worth listing either way: per-session exclusivity (#94595) + # made the registry authoritative for every surface, so with no cap set the + # entries exist and only this readout was still gated on the cap. try: from hermes_cli.active_sessions import ( active_session_registry_snapshot, + current_repo_root, format_age, resolve_max_concurrent_sessions, ) _cap = resolve_max_concurrent_sessions(config) - except Exception: - _cap = None - if _cap: try: _held = active_session_registry_snapshot() except Exception: _held = [] + _here = current_repo_root() + except Exception: + _cap = None + _held = [] + _here = None + if _cap: _full = len(_held) >= _cap print( " Slots: " @@ -689,12 +696,26 @@ def _resolve_env(env_ref) -> str: f"{len(_held)}/{_cap} in use", Colors.YELLOW if _full else Colors.GREEN ) ) + elif _held: + # No cap means no slots to report, but "another session is live in this + # checkout" is exactly the signal #46303 asks for. + print(" Live: " + color(f"{len(_held)} session(s)", Colors.GREEN)) + if _held: _now = time.time() for _entry in sorted(_held, key=lambda e: e.get("started_at") or 0): _age = format_age(_now - float(_entry.get("started_at") or _now)) + _meta = _entry.get("metadata") + _repo = (_meta or {}).get("repo_root") if isinstance(_meta, dict) else None + _where = "" + if _repo: + _where = ( + " ← this repo" + if _here and str(_repo) == str(_here) + else f" {os.path.basename(str(_repo))}" + ) print( f" {_entry.get('surface') or 'unknown':<17} " - f"{_entry.get('session_id') or '?':<24} {_age}" + f"{_entry.get('session_id') or '?':<24} {_age}{_where}" ) # ========================================================================= diff --git a/tests/hermes_cli/test_active_sessions.py b/tests/hermes_cli/test_active_sessions.py index 0fe223101fc2..5c7b7fdf90b4 100644 --- a/tests/hermes_cli/test_active_sessions.py +++ b/tests/hermes_cli/test_active_sessions.py @@ -646,3 +646,75 @@ def test_liveness_guard_keeps_a_just_acquired_own_lease_it_cannot_vouch_for( ) as active: assert active is False assert active_sessions.active_session_registry_snapshot(home) == [] + + +def test_entries_carry_repo_root_so_sessions_are_attributable(tmp_path, monkeypatch): + """A registry entry with no repo dimension cannot answer the #46303 question. + + "Is another session already attached to *this* checkout?" needs the checkout + on the entry; surface and session id alone do not say where a session is + working. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + repo_a = tmp_path / "repo_a" + (repo_a / ".git").mkdir(parents=True) + repo_b = tmp_path / "repo_b" + (repo_b / ".git").mkdir(parents=True) + + monkeypatch.chdir(repo_a) + active_sessions.try_acquire_active_session( + session_id="in_a", surface="cli", config={} + ) + monkeypatch.chdir(repo_b) + active_sessions.try_acquire_active_session( + session_id="in_b", surface="desktop", config={} + ) + + attributed = { + entry["session_id"]: (entry.get("metadata") or {}).get("repo_root") + for entry in active_sessions.active_session_registry_snapshot() + } + assert attributed == { + "in_a": str(repo_a.resolve()), + "in_b": str(repo_b.resolve()), + } + + +def test_repo_root_is_omitted_outside_any_checkout(tmp_path, monkeypatch): + """Attribution is advisory: no checkout means no field, not a null one.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + elsewhere = tmp_path / "not_a_repo" + elsewhere.mkdir() + monkeypatch.chdir(elsewhere) + + lease, message = active_sessions.try_acquire_active_session( + session_id="homeless", surface="cli", config={} + ) + assert lease is not None and message is None + + (entry,) = active_sessions.active_session_registry_snapshot() + assert "repo_root" not in (entry.get("metadata") or {}) + + +def test_repo_root_survives_a_session_id_transfer(tmp_path, monkeypatch): + """A transfer re-writes metadata from the caller's identity fields, which + would otherwise silently drop the attribution the entry already had — the + process has not changed repos just because the session got an id. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + repo = tmp_path / "repo" + (repo / ".git").mkdir(parents=True) + monkeypatch.chdir(repo) + + lease, message = active_sessions.try_acquire_active_session( + session_id="draft", surface="cli", config={} + ) + assert lease is not None and message is None + + assert active_sessions.transfer_active_session( + lease, session_id="saved", metadata={"live_session_id": "saved"} + ) + + (entry,) = active_sessions.active_session_registry_snapshot() + assert entry["session_id"] == "saved" + assert entry["metadata"]["repo_root"] == str(repo.resolve()) diff --git a/tests/hermes_cli/test_status.py b/tests/hermes_cli/test_status.py index 37d0c0bb243c..70b6ba578919 100644 --- a/tests/hermes_cli/test_status.py +++ b/tests/hermes_cli/test_status.py @@ -249,3 +249,59 @@ def close(self): assert "Active: 2 session(s)" in output assert "Last activity:" in output assert "1m ago" in output + + +def test_show_status_lists_live_sessions_without_a_cap(monkeypatch, capsys, tmp_path): + """Per-session exclusivity (#94595) made the registry authoritative for every + surface, capped or not — but this readout was still gated on the cap, so an + operator who never set ``max_concurrent_sessions`` saw nothing at all even + though the entries were right there. That is the awareness signal #46303 + asks for. + """ + from hermes_cli import active_sessions + + home = tmp_path / ".hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + repo = tmp_path / "checkout" + (repo / ".git").mkdir(parents=True) + monkeypatch.chdir(repo) + + lease, message = active_sessions.try_acquire_active_session( + session_id="20260614_105454_7afa2b", surface="desktop", config={} + ) + assert lease is not None and message is None + + show_status(SimpleNamespace(all=False, deep=False)) + + output = capsys.readouterr().out + assert "Live:" in output + assert "1 session(s)" in output + assert "20260614_105454_7afa2b" in output + assert "desktop" in output + # Attribution: the session was started in this checkout, so say so. + assert "this repo" in output + assert "Slots:" not in output + + +def test_show_status_still_reports_slots_when_a_cap_is_set(monkeypatch, capsys, tmp_path): + """The uncapped listing must not displace the capped slot accounting.""" + from hermes_cli import active_sessions + from hermes_cli import status as status_mod + + home = tmp_path / ".hermes" + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr( + status_mod, "load_config", lambda: {"max_concurrent_sessions": 2}, raising=False + ) + + lease, message = active_sessions.try_acquire_active_session( + session_id="capped-1", surface="cli", config={"max_concurrent_sessions": 2} + ) + assert lease is not None and message is None + + show_status(SimpleNamespace(all=False, deep=False)) + + output = capsys.readouterr().out + assert "1/2 in use" in output + assert "capped-1" in output + assert "Live:" not in output