From e19c67adde291d92d01d58aed70456c17a2db2d0 Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes da Silva Date: Fri, 14 Aug 2026 22:11:05 -0300 Subject: [PATCH] refactor(sessions): resolve project ownership through _profiles_match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit profile_scoped_project_ids() duplicated the body of the canonical api/profiles.py::_profiles_match, whose docstring says it was relocated there precisely so callers stop duplicating it. Call it instead. Behaviour-identical: both spellings agree on all twelve None / '' / 'default' / renamed-root / foreign-profile combinations, which the new parametrised test asserts cell for cell against _profiles_match itself. It is a drift guard, not a regression test — swapping the call back to a raw `p.get('profile') == active` comparison reddens seven of its twelve cells. --- api/models.py | 16 +++--- ...test_project_assigned_cli_session_limit.py | 49 +++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/api/models.py b/api/models.py index 2616563a..985caaca 100644 --- a/api/models.py +++ b/api/models.py @@ -7369,21 +7369,21 @@ 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/alias matching mirrors ``ensure_cron_project`` so the two never - disagree about which profile owns a project. Read-only. + 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 + project (renamed root, legacy ``'default'`` tag, and a missing ``profile`` + key are all its business, not ours). Read-only. """ - from api.profiles import get_active_profile_name, _is_root_profile + from api.profiles import get_active_profile_name, _profiles_match - active = get_active_profile_name() or 'default' + active = get_active_profile_name() resolved: set[str] = set() for p in load_projects(): project_id = str(p.get('project_id') or '').strip() if not project_id: continue - row_profile = p.get('profile') - if row_profile == active or ( - _is_root_profile(row_profile or 'default') and _is_root_profile(active) - ): + if _profiles_match(p.get('profile'), active): resolved.add(project_id) return frozenset(resolved) diff --git a/tests/test_project_assigned_cli_session_limit.py b/tests/test_project_assigned_cli_session_limit.py index 33c39a24..2edc0fab 100755 --- a/tests/test_project_assigned_cli_session_limit.py +++ b/tests/test_project_assigned_cli_session_limit.py @@ -531,6 +531,55 @@ def test_unresolvable_project_id_leaves_the_session_unassigned( assert by_id["old-live"]["project_id"] == "project-live" +@pytest.mark.parametrize( + "row_profile, active, expected", + [ + (None, None, True), + ("", None, True), + ("default", None, True), + (None, "default", True), + ("", "default", True), + ("default", "default", True), + # Renamed root: the legacy 'default' tag and the display name are the + # same profile, in both directions. + ("kinni", "default", True), + ("default", "kinni", True), + (None, "kinni", True), + # Genuinely different profiles never cross. + ("other", "default", False), + ("default", "other", False), + ("other", "other", True), + ], +) +def test_profile_scoped_project_ids_uses_the_canonical_profile_match( + fake_hermes_home, tmp_path, monkeypatch, row_profile, active, expected +): + """Ownership is decided by ``_profiles_match``, not by a local copy of it. + + ``api/profiles.py::_profiles_match`` documents that it exists so callers + stop duplicating this None/''/'default'/renamed-root matrix. Each row asserts + the catalog read agrees with the canonical helper cell for cell, so the two + cannot drift apart the way the duplicated body could. + """ + import api.profiles as profiles + + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: active) + monkeypatch.setattr( + profiles, "_is_root_profile", lambda name: name in {"default", "kinni"} + ) + (tmp_path / "projects.json").write_text( + json.dumps([ + {"project_id": "p1", "name": "P1", "color": "#000", + "profile": row_profile, "created_at": 1.0}, + ]), + encoding="utf-8", + ) + models.clear_cli_sessions_cache() + + assert profiles._profiles_match(row_profile, active) is expected + assert ("p1" in models.profile_scoped_project_ids()) is expected + + def test_unresolvable_project_overflow_is_not_marked_hidden(fake_hermes_home, tmp_path): """End of the same chain: routes must see None, so no orphan default_hidden.""" _register_projects(tmp_path, "project-live")