From 084f70df360a67fc458ff63874d37418ae620fc0 Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes da Silva Date: Sun, 23 Aug 2026 21:03:44 -0300 Subject: [PATCH] fix(sessions): scope project-id resolution to the session's profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review asked for "confirming the project still exists for the session's profile". profile_scoped_project_ids() read get_active_profile_name(), but _load_cli_sessions_uncached runs once per profile context in the ?all_profiles=1 view, so every context resolved against the ACTIVE profile's catalog. A LIVE assignment owned by another profile was therefore misread as unresolvable: coerced to None, skipped by the assigned recovery pass, and unreachable by the unassigned refill too (its state.db row does carry a project_id, which the "unassigned" SQL filter excludes), so the row left the payload entirely — while its chip stayed selectable in that same view, because /api/projects returns every project when all_profiles=1. Thread the ctx_profile already available at the call site through to the catalog read. The safe direction is unchanged: an id that belongs to a different profile, or to no project at all, still coerces to None. --- api/models.py | 34 ++++++++--- ...test_project_assigned_cli_session_limit.py | 58 +++++++++++++++++++ 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/api/models.py b/api/models.py index c89a91ac..213317f8 100644 --- a/api/models.py +++ b/api/models.py @@ -7372,8 +7372,14 @@ def _profile_has_user_projects(profile: str | None = None) -> bool: return False -def profile_scoped_project_ids() -> frozenset[str]: - """Project IDs that currently exist for the active profile. +# Sentinel for profile_scoped_project_ids(): "no profile given, use the active +# one". A bare None cannot mean that, because None is a legitimate profile value +# (_profiles_match reads it as the root profile). +_ACTIVE_PROFILE = object() + + +def profile_scoped_project_ids(profile=_ACTIVE_PROFILE) -> frozenset[str]: + """Project IDs that currently exist for ``profile`` (default: the active one). A state.db ``project_id`` is an opaque string the agent wrote; the project it names can since have been deleted, or can belong to a different profile. @@ -7382,6 +7388,13 @@ def profile_scoped_project_ids() -> frozenset[str]: the row out of "Unassigned" and turn it into a ``default_hidden`` row with no project chip left to reveal it (#6659 review finding 3). + ``profile`` must be the SESSION's profile, not the process-wide active one, + wherever the two can differ. The all-profiles sidebar view scans one state.db + per profile in a single request, so resolving every context against the active + profile made another profile's LIVE assignment look unresolvable and dropped + the row from the payload entirely — while its chip stayed selectable in that + same view (#6659 review finding 3). + Profile/alias matching is delegated to ``api.profiles._profiles_match``, the canonical helper every other profile-scoped read already uses, so this set cannot disagree with the rest of the app about which profile owns a @@ -7390,7 +7403,7 @@ def profile_scoped_project_ids() -> frozenset[str]: """ from api.profiles import get_active_profile_name, _profiles_match - active = get_active_profile_name() + active = get_active_profile_name() if profile is _ACTIVE_PROFILE else profile resolved: set[str] = set() for p in load_projects(): project_id = str(p.get('project_id') or '').strip() @@ -8392,15 +8405,22 @@ def _load_cli_sessions_uncached( _webhook_pid_cache[0] = ensure_webhook_project(profile=_cli_profile) return _webhook_pid_cache[0] - # The live project catalog for this profile, read lazily and at most once per - # scan (same [resolved, value] cell pattern as _cron_pid above, for the same - # #4842 reason: a per-row load_projects() is a cold-sidebar I/O blowup). + # The live project catalog for THIS SCAN'S profile, read lazily and at most + # once per scan (same [resolved, value] cell pattern as _cron_pid above, for + # the same #4842 reason: a per-row load_projects() is a cold-sidebar I/O + # blowup). + # + # Scoped to ``_cli_profile`` — the profile whose state.db this scan is + # reading — not to the process-wide active profile. The all-profiles view + # calls this loader once per profile context in a single request, so the + # active-profile catalog would misread another profile's LIVE assignment as + # unresolvable and drop the row from the payload (#6659 review finding 3). _known_project_ids_cache: list = [False, frozenset()] def _known_project_ids() -> frozenset[str]: if not _known_project_ids_cache[0]: _known_project_ids_cache[0] = True try: - _known_project_ids_cache[1] = profile_scoped_project_ids() + _known_project_ids_cache[1] = profile_scoped_project_ids(_cli_profile) except Exception: logger.debug("Project catalog read failed for CLI projection", exc_info=True) return _known_project_ids_cache[1] diff --git a/tests/test_project_assigned_cli_session_limit.py b/tests/test_project_assigned_cli_session_limit.py index e0d1bf2a..afa8971f 100755 --- a/tests/test_project_assigned_cli_session_limit.py +++ b/tests/test_project_assigned_cli_session_limit.py @@ -1695,3 +1695,61 @@ def test_all_profiles_view_recovers_project_assigned_sessions( assert [s["session_id"] for s in sessions].count("older-assigned") == 1 # The unassigned window is still bounded — the recovery pass is additive. assert sum(1 for s in sessions if s["project_id"] is None) == 5 + + +def test_all_profiles_view_resolves_each_context_against_its_own_profile( + fake_hermes_home, tmp_path, monkeypatch +): + """Review finding 3 asked for "the SESSION's profile", not the active one. + + ``_load_cli_sessions_uncached`` runs once per profile context here, but the + project catalog it resolved against came from ``get_active_profile_name()``. + So a LIVE assignment owned by another profile was misread as unresolvable: + coerced to None, skipped by the assigned recovery pass, and unreachable by the + unassigned refill too (its state.db row DOES carry a ``project_id``, which the + "unassigned" SQL filter excludes) — the row left the payload entirely. Its + chip is selectable in this very view (``/api/projects`` returns every project + when ``all_profiles=1``), so selecting it showed nothing: exactly the + "assignment exists but the session is unreachable" failure finding 3 named. + """ + monkeypatch.setattr(models, "CLI_VISIBLE_SESSION_LIMIT", 5) + _register_projects(tmp_path, "project-a") + _register_projects(tmp_path, "project-b", profile="other") + + home_a = fake_hermes_home + db_a = home_a / "state.db" + rows_a = [_session(f"a-recent-{index:02d}", BASE_TS + 100 + index) for index in range(25)] + rows_a.append(_session("older-assigned-a", BASE_TS, project_id="project-a")) + _write_state_db(db_a, rows_a) + + home_b = tmp_path / "hermes-other" + home_b.mkdir() + db_b = home_b / "state.db" + rows_b = [_session(f"b-recent-{index:02d}", BASE_TS + 100 + index) for index in range(25)] + rows_b.append(_session("older-assigned-b", BASE_TS, project_id="project-b")) + # The safe direction, in the same scan: project-a belongs to the OTHER + # profile as far as this context is concerned, so it must still coerce to + # None instead of gaining a chip that cannot reveal it. + rows_b.append(_session("newest-foreign-b", BASE_TS + 900, project_id="project-a")) + _write_state_db(db_b, rows_b) + + # Enumerating real profiles shells out to the agent CLI; pin the two contexts. + monkeypatch.setattr( + models, + "_all_profiles_cli_contexts", + lambda: ( + [(home_a, db_a, "default"), (home_b, db_b, "other")], + (("home-a", "default", 1), ("home-b", "other", 1)), + ), + ) + + sessions = models.get_cli_sessions(all_profiles=True) + by_id = {session["session_id"]: session for session in sessions} + + # The active profile's own context is unaffected. + assert by_id["older-assigned-a"]["project_id"] == "project-a" + # The other profile's LIVE assignment survives, with its own chip. + assert "older-assigned-b" in by_id + assert by_id["older-assigned-b"]["project_id"] == "project-b" + # A foreign id is still not an assignment. + assert by_id["newest-foreign-b"]["project_id"] is None