From 8e398cd1b32a186c87a36ced26425081d36fd77a Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Wed, 1 Jul 2026 23:29:16 -0400 Subject: [PATCH] fix(cron): stop auto-creating the Cron Jobs project without project opt-in (#5379) --- api/models.py | 55 ++- api/routes.py | 6 +- .../test_issue3930_source_filter_pushdown.py | 4 +- .../test_issue4385_cron_archive_reappears.py | 2 +- tests/test_issue5379_cron_project_optin.py | 400 ++++++++++++++++++ 5 files changed, 456 insertions(+), 11 deletions(-) create mode 100644 tests/test_issue5379_cron_project_optin.py diff --git a/api/models.py b/api/models.py index 45bffd1a..d5408c9f 100644 --- a/api/models.py +++ b/api/models.py @@ -4573,7 +4573,7 @@ CRON_PROJECT_NAME = 'Cron Jobs' _CRON_PROJECT_LOCK = threading.Lock() -def ensure_cron_project() -> str: +def ensure_cron_project(create: bool = True) -> str | None: """Return the project_id of the system "Cron Jobs" project for the active profile. Each profile gets its own "Cron Jobs" project so cron-spawned sessions in @@ -4582,7 +4582,16 @@ def ensure_cron_project() -> str: `profile` field) is treated as belonging to whichever profile first calls this in a given install, then re-tagged. - Thread-safe and idempotent. Returns a 12-char hex project_id string. + When `create` is False, only an EXISTING per-profile cron project is + resolved (exact tag, renamed-root alias, or legacy-untagged back-tag); + no new project is minted and None is returned instead. Callers gate + `create` on `_profile_has_user_projects()` so cron sessions don't force + a "Cron Jobs" chip onto installs that never opted into project + organization (#5379). Direct callers that omit `create` keep today's + unconditional-create behavior. + + Thread-safe and idempotent. Returns a 12-char hex project_id string, or + None if `create` is False and no existing cron project resolves. """ from api.profiles import get_active_profile_name, _is_root_profile @@ -4607,6 +4616,8 @@ def ensure_cron_project() -> str: p['profile'] = active save_projects(projects) return p['project_id'] + if not create: + return None # Otherwise create a new one tagged with the active profile. project_id = uuid.uuid4().hex[:12] projects.append({ @@ -4656,6 +4667,32 @@ def ensure_webhook_project() -> str: return project_id +def _profile_has_user_projects() -> bool: + """True if the active profile already has at least one real (non-system) project. + + "Opted into project organization" means `load_projects()` contains a + project whose name is not a reserved system name (`CRON_PROJECT_NAME`, + `WEBHOOK_PROJECT_NAME`), tagged to the active profile or its renamed-root + alias. Profile/alias matching mirrors `ensure_cron_project`'s own lookup + so the two never disagree about which profile a project belongs to. + + Read-only: never mutates projects.json, safe to call as often as needed. + """ + from api.profiles import get_active_profile_name, _is_root_profile + + active = get_active_profile_name() or 'default' + reserved = {CRON_PROJECT_NAME, WEBHOOK_PROJECT_NAME} + for p in load_projects(): + if p.get('name') in reserved: + continue + row_profile = p.get('profile') + if row_profile == active: + return True + if _is_root_profile(row_profile or 'default') and _is_root_profile(active): + return True + return False + + def is_cron_session(session_id: str, source_tag: str | None = None) -> bool: """Return True if a session originates from a cron job.""" if source_tag == 'cron': @@ -5553,11 +5590,17 @@ def _load_cli_sessions_uncached( # Memoize the cron project ID for this scan so we don't pay a lock-acquire + # disk-read of projects.json per cron session in the loop below. # Resolved lazily on the first cron session we encounter. - _cron_pid_cache = [None] # list-as-cell so the closure can mutate + # [resolved, project_id_or_None] — a plain `[None]` sentinel can't tell + # "not yet resolved" apart from "resolved to None" (the gated-closed + # case), which would re-pay the load_projects() read on every cron row + # in a cron-heavy zero-user-project scan — the exact I/O blowup #4842 + # fixed, reintroduced by this gate if left as a bare None check. + _cron_pid_cache: list = [False, None] def _cron_pid(): - if _cron_pid_cache[0] is None: - _cron_pid_cache[0] = ensure_cron_project() - return _cron_pid_cache[0] + if not _cron_pid_cache[0]: + _cron_pid_cache[0] = True + _cron_pid_cache[1] = ensure_cron_project(create=_profile_has_user_projects()) + return _cron_pid_cache[1] # Memoize the cron jobs.json job_id -> name map for this scan. The two row # loops below each looked up a cron job's friendly name by re-reading and diff --git a/api/routes.py b/api/routes.py index 132ad5e9..5d39c95f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -8565,6 +8565,7 @@ from api.models import ( _record_webui_zero_message_orphan_tombstone, _clear_webui_zero_message_orphan_tombstone, ensure_cron_project, + _profile_has_user_projects, is_cron_session, is_safe_session_id, ) @@ -23255,10 +23256,11 @@ def _handle_session_import_cli(handler, body): # Use the CLI session title if available (e.g., cron job name), otherwise derive from messages title = cli_title or title_from(msgs, "CLI Session") - # Auto-assign cron sessions to the dedicated "Cron Jobs" project (#1079) + # Auto-assign cron sessions to the dedicated "Cron Jobs" project (#1079), + # gated on whether this profile has opted into project organization (#5379) cron_project_id = None if is_cron_session(sid, cli_source_tag): - cron_project_id = ensure_cron_project() + cron_project_id = ensure_cron_project(create=_profile_has_user_projects()) if _read_only_view: session_payload = { diff --git a/tests/test_issue3930_source_filter_pushdown.py b/tests/test_issue3930_source_filter_pushdown.py index fc4c35ce..2cd93292 100644 --- a/tests/test_issue3930_source_filter_pushdown.py +++ b/tests/test_issue3930_source_filter_pushdown.py @@ -242,7 +242,7 @@ def test_load_cli_sessions_uncached_pushes_specific_source_into_state_db_scan(mo monkeypatch.setattr(models, "get_claude_code_sessions", lambda: claude_calls.append(True) or []) monkeypatch.setattr(models, "read_importable_agent_session_rows", fake_read_rows) monkeypatch.setattr(models, "get_last_workspace", lambda: tmp_path) - monkeypatch.setattr(models, "ensure_cron_project", lambda: "cron-project-id") + monkeypatch.setattr(models, "ensure_cron_project", lambda **_: "cron-project-id") monkeypatch.setattr(models.Session, "load_metadata_only", lambda _sid: None) result = models._load_cli_sessions_uncached(tmp_path, db, _cli_profile=None, source_filter="tui") @@ -283,7 +283,7 @@ def test_cron_source_filter_uses_cron_rescue_limit(monkeypatch, tmp_path): monkeypatch.setattr(models, "read_importable_agent_session_rows", fake_read_rows) monkeypatch.setattr(models, "get_last_workspace", lambda: tmp_path) - monkeypatch.setattr(models, "ensure_cron_project", lambda: "cron-project-id") + monkeypatch.setattr(models, "ensure_cron_project", lambda **_: "cron-project-id") monkeypatch.setattr(models.Session, "load_metadata_only", lambda _sid: None) result = models._load_cli_sessions_uncached(tmp_path, db, _cli_profile=None, source_filter="cron") diff --git a/tests/test_issue4385_cron_archive_reappears.py b/tests/test_issue4385_cron_archive_reappears.py index d91a5695..a585e6f7 100644 --- a/tests/test_issue4385_cron_archive_reappears.py +++ b/tests/test_issue4385_cron_archive_reappears.py @@ -123,7 +123,7 @@ def test_cron_state_projection_preserves_archived_sidecar(monkeypatch, tmp_path) encoding="utf-8", ) monkeypatch.setattr(models, "SESSION_DIR", session_dir) - monkeypatch.setattr(models, "ensure_cron_project", lambda: "cron-project") + monkeypatch.setattr(models, "ensure_cron_project", lambda **_: "cron-project") models.clear_sidecar_metadata_cache() rows = models._load_cli_sessions_uncached( diff --git a/tests/test_issue5379_cron_project_optin.py b/tests/test_issue5379_cron_project_optin.py new file mode 100644 index 00000000..c3069a86 --- /dev/null +++ b/tests/test_issue5379_cron_project_optin.py @@ -0,0 +1,400 @@ +"""Regression tests for #5379: gate 'Cron Jobs' project auto-creation behind +project opt-in. + +ensure_cron_project() used to unconditionally mint and persist a 'Cron Jobs' +project the first time any code path saw a cron session, even on installs +that never created a project of their own. These tests pin the fix: creation +is now gated on whether the active profile already has at least one real +(non-system) project; lookup, renamed-root alias resolution, and legacy +untagged back-tagging are all unaffected. ensure_cron_project() itself is +never mocked here — only PROJECTS_FILE and profile-resolution helpers are. +""" + +import json +import sqlite3 +import threading +import time + +import pytest + + +def _make_cron_state_db(path, *, cron_count=1): + """state.db with a batch of cron-sourced sessions (mirrors #4842's helper).""" + conn = sqlite3.connect(str(path)) + conn.executescript( + """ + CREATE TABLE sessions ( + id TEXT PRIMARY KEY, + source TEXT, + session_source TEXT, + title TEXT, + model TEXT, + started_at REAL NOT NULL, + message_count INTEGER DEFAULT 0, + parent_session_id TEXT, + ended_at REAL, + end_reason TEXT + ); + CREATE TABLE messages ( + id TEXT PRIMARY KEY, + session_id TEXT, + role TEXT, + content TEXT, + timestamp REAL + ); + """ + ) + now = time.time() + for i in range(cron_count): + sid = f"cron_job{i:04d}_{int(now) + i}" + conn.execute( + "INSERT INTO sessions (id, source, session_source, title, model," + " started_at, message_count, parent_session_id, ended_at, end_reason)" + " VALUES (?, 'cron', 'cron', 'Cron run', 'test-model', ?, 1, NULL, NULL, NULL)", + (sid, now + i), + ) + conn.execute( + "INSERT INTO messages (id, session_id, role, content, timestamp)" + " VALUES (?, ?, 'user', 'cron task', ?)", + (f"cron_msg_{i:04d}", sid, now + i), + ) + conn.commit() + conn.close() + + +def _write_projects(path, projects): + path.write_text(json.dumps(projects), encoding="utf-8") + + +@pytest.fixture(autouse=True) +def _isolate_projects(tmp_path, monkeypatch): + """Point PROJECTS_FILE at a fresh tmp_path file; reset profile-alias caches.""" + import api.config as cfg + import api.models as models + import api.profiles as profiles + + projects_file = tmp_path / "projects.json" + monkeypatch.setattr(cfg, "PROJECTS_FILE", projects_file) + monkeypatch.setattr(models, "PROJECTS_FILE", projects_file) + monkeypatch.setattr(models, "_projects_migrated", True) + monkeypatch.setattr(models, "_CRON_PROJECT_LOCK", threading.Lock()) + monkeypatch.setattr(models, "_WEBHOOK_PROJECT_LOCK", threading.Lock()) + monkeypatch.setattr(profiles, "list_profiles_api", lambda: []) + monkeypatch.setattr(profiles, "_active_profile", "default") + profiles._invalidate_root_profile_cache() + yield projects_file + profiles._invalidate_root_profile_cache() + + +def test_sidebar_scan_zero_user_projects_skips_cron_project_creation(tmp_path): + """Reproduction anchor: listing the sidebar with a cron row must not + create or persist a Cron Jobs project when no user project exists.""" + import api.models as models + + projects_file = tmp_path / "projects.json" + db = tmp_path / "state.db" + _make_cron_state_db(db, cron_count=1) + + rows = models._load_cli_sessions_uncached( + tmp_path, db, _cli_profile=None, include_claude_code=False, + ) + + cron_rows = [r for r in rows if r["source_tag"] == "cron"] + assert len(cron_rows) == 1 + assert cron_rows[0]["project_id"] is None + assert not projects_file.exists(), ( + "gate must not persist a Cron Jobs project when the active profile " + "has zero user-created projects" + ) + + +def test_recovery_endpoint_zero_user_projects_skips_cron_project_creation(monkeypatch, tmp_path): + import api.routes as routes + + sid = "cron_recover_0001" + cron_meta = { + "session_id": sid, + "title": "Cron run", + "model": "test-model", + "created_at": 10.0, + "updated_at": 20.0, + "source_tag": "cron", + "raw_source": "cron", + "session_source": "cron", + "source_label": "Cron", + "is_cli_session": True, + "read_only": False, + } + messages = [{"role": "user", "content": "run"}, {"role": "assistant", "content": "done"}] + + class FakeSession: + def __init__(self): + self.project_id = None + + def save(self, touch_updated_at=False): + pass + + def compact(self): + return {"session_id": sid, "title": "Cron run"} + + fake = FakeSession() + + monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, _sid: None)) + monkeypatch.setattr(routes, "require", lambda body, *keys: None) + monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: messages) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [cron_meta]) + monkeypatch.setattr(routes, "import_cli_session", lambda *a, **k: fake) + monkeypatch.setattr(routes, "publish_session_list_changed", lambda *a, **k: None) + monkeypatch.setattr(routes, "_queue_generated_title_for_imported_session", lambda *a, **k: None) + + response = routes._handle_session_import_cli(object(), {"session_id": sid}) + + assert response["imported"] is True + assert fake.project_id is None + assert not (tmp_path / "projects.json").exists() + + +def test_sidebar_scan_with_user_project_still_autoassigns_cron(tmp_path): + import api.models as models + + projects_file = tmp_path / "projects.json" + _write_projects(projects_file, [ + {"project_id": "user-proj-1", "name": "My Website", "profile": "default", "created_at": 1.0}, + ]) + db = tmp_path / "state.db" + _make_cron_state_db(db, cron_count=1) + + rows = models._load_cli_sessions_uncached( + tmp_path, db, _cli_profile=None, include_claude_code=False, + ) + + cron_rows = [r for r in rows if r["source_tag"] == "cron"] + assert len(cron_rows) == 1 + assert cron_rows[0]["project_id"] is not None + + saved = json.loads(projects_file.read_text()) + cron_saved = [p for p in saved if p["name"] == "Cron Jobs"] + assert len(cron_saved) == 1 + + +def test_recovery_endpoint_with_user_project_still_autoassigns_cron(monkeypatch, tmp_path): + import api.routes as routes + + projects_file = tmp_path / "projects.json" + _write_projects(projects_file, [ + {"project_id": "user-proj-1", "name": "My Website", "profile": "default", "created_at": 1.0}, + ]) + + sid = "cron_recover_0002" + cron_meta = { + "session_id": sid, + "title": "Cron run", + "model": "test-model", + "created_at": 10.0, + "updated_at": 20.0, + "source_tag": "cron", + "raw_source": "cron", + "session_source": "cron", + "source_label": "Cron", + "is_cli_session": True, + "read_only": False, + } + messages = [{"role": "user", "content": "run"}, {"role": "assistant", "content": "done"}] + + class FakeSession: + def __init__(self): + self.project_id = None + + def save(self, touch_updated_at=False): + pass + + def compact(self): + return {"session_id": sid, "title": "Cron run"} + + fake = FakeSession() + + monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, _sid: None)) + monkeypatch.setattr(routes, "require", lambda body, *keys: None) + monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda _sid, profile=None: messages) + monkeypatch.setattr(routes, "get_cli_sessions", lambda source_filter=None, all_profiles=False: [cron_meta]) + monkeypatch.setattr(routes, "import_cli_session", lambda *a, **k: fake) + monkeypatch.setattr(routes, "publish_session_list_changed", lambda *a, **k: None) + monkeypatch.setattr(routes, "_queue_generated_title_for_imported_session", lambda *a, **k: None) + + response = routes._handle_session_import_cli(object(), {"session_id": sid}) + + assert response["imported"] is True + assert fake.project_id is not None + saved = json.loads(projects_file.read_text()) + assert any(p["name"] == "Cron Jobs" for p in saved) + + +def test_preexisting_tagged_cron_project_resolves_when_zero_user_projects(tmp_path): + import api.models as models + + projects_file = tmp_path / "projects.json" + _write_projects(projects_file, [ + {"project_id": "existing-cron", "name": "Cron Jobs", "profile": "default", "color": "#6366f1", "created_at": 1.0}, + ]) + db = tmp_path / "state.db" + _make_cron_state_db(db, cron_count=1) + + rows = models._load_cli_sessions_uncached( + tmp_path, db, _cli_profile=None, include_claude_code=False, + ) + + cron_rows = [r for r in rows if r["source_tag"] == "cron"] + assert cron_rows[0]["project_id"] == "existing-cron" + saved = json.loads(projects_file.read_text()) + assert len(saved) == 1 + assert saved[0]["project_id"] == "existing-cron" + + +def test_legacy_untagged_cron_project_back_tagged_when_zero_user_projects(tmp_path, monkeypatch): + import api.models as models + import api.profiles as profiles + + # A non-default active profile is required to exercise the back-tag loop: + # with 'default' active, an untagged row's implicit profile and the active + # profile both alias to the root profile (_is_root_profile('default') is + # unconditionally True), so the FIRST (alias-match) lookup loop would + # already resolve it without ever reaching the back-tag loop below it. + # Mirrors the active-profile choice in the proven + # test_issue1614_project_profile_filtering.py::test_ensure_cron_project_back_tags_legacy_untagged. + monkeypatch.setattr(profiles, "_active_profile", "haku") + profiles._invalidate_root_profile_cache() + + projects_file = tmp_path / "projects.json" + _write_projects(projects_file, [ + {"project_id": "legacy-cron", "name": "Cron Jobs", "color": "#6366f1", "created_at": 1.0}, + ]) + db = tmp_path / "state.db" + _make_cron_state_db(db, cron_count=1) + + rows = models._load_cli_sessions_uncached( + tmp_path, db, _cli_profile=None, include_claude_code=False, + ) + + cron_rows = [r for r in rows if r["source_tag"] == "cron"] + assert cron_rows[0]["project_id"] == "legacy-cron" + saved = json.loads(projects_file.read_text()) + assert saved[0]["profile"] == "haku", ( + "legacy untagged project must still be back-tagged even when the " + "create branch is gated off" + ) + + +def test_renamed_root_alias_cron_project_resolves_when_zero_user_projects(tmp_path, monkeypatch): + import api.models as models + import api.profiles as profiles + + projects_file = tmp_path / "projects.json" + _write_projects(projects_file, [ + {"project_id": "root-alias-cron", "name": "Cron Jobs", "profile": "default", "color": "#6366f1", "created_at": 1.0}, + ]) + monkeypatch.setattr(profiles, "list_profiles_api", lambda: [ + {"name": "kinni", "is_default": True, "path": str(tmp_path)}, + ]) + monkeypatch.setattr(profiles, "_active_profile", "kinni") + profiles._invalidate_root_profile_cache() + + db = tmp_path / "state.db" + _make_cron_state_db(db, cron_count=1) + + rows = models._load_cli_sessions_uncached( + tmp_path, db, _cli_profile=None, include_claude_code=False, + ) + + cron_rows = [r for r in rows if r["source_tag"] == "cron"] + assert cron_rows[0]["project_id"] == "root-alias-cron" + + +def test_webhook_project_still_auto_created_when_zero_user_projects(tmp_path): + """Adjacent-condition row: the ungated webhook path must still + auto-create its dedicated project even in the same zero-user-project + scan where the cron path is now suppressed.""" + import api.models as models + + projects_file = tmp_path / "projects.json" + db_path = tmp_path / "state.db" + with sqlite3.connect(db_path) as conn: + conn.execute( + """ + CREATE TABLE sessions ( + id TEXT PRIMARY KEY, title TEXT, model TEXT, message_count INTEGER, + started_at REAL, source TEXT, user_id TEXT + ) + """ + ) + conn.execute( + """ + CREATE TABLE messages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, session_id TEXT NOT NULL, + role TEXT NOT NULL, content TEXT, timestamp REAL NOT NULL + ) + """ + ) + conn.execute( + "INSERT INTO sessions (id, title, model, message_count, started_at, source, user_id)" + " VALUES ('webhook_route_1', NULL, 'test-model', 1, 20, 'webhook', 'webhook:read-later')" + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp)" + " VALUES ('webhook_route_1', 'user', 'payload', 21)" + ) + + rows = models._load_cli_sessions_uncached( + tmp_path, db_path, "default", include_claude_code=False, + ) + + webhook_rows = [r for r in rows if r["source_tag"] == "webhook"] + assert len(webhook_rows) == 1 + assert webhook_rows[0]["project_id"] is not None + + saved = json.loads(projects_file.read_text()) + assert any(p["name"] == "Webhooks" for p in saved) + + +def test_profile_has_user_projects_ignores_system_named_projects(tmp_path): + import api.models as models + + projects_file = tmp_path / "projects.json" + + _write_projects(projects_file, []) + assert models._profile_has_user_projects() is False + + _write_projects(projects_file, [ + {"project_id": "c1", "name": "Cron Jobs", "profile": "default", "created_at": 1.0}, + {"project_id": "w1", "name": "Webhooks", "profile": "default", "created_at": 1.0}, + ]) + assert models._profile_has_user_projects() is False, ( + "system-named projects don't count as opting in" + ) + + _write_projects(projects_file, [ + {"project_id": "a1", "name": "Cron Jobs Archive", "profile": "default", "created_at": 1.0}, + ]) + assert models._profile_has_user_projects() is True, ( + "only exact reserved names are excluded; similar names are user projects" + ) + + _write_projects(projects_file, [ + {"project_id": "c1", "name": "Cron Jobs", "profile": "default", "created_at": 1.0}, + {"project_id": "u1", "name": "My Real Project", "profile": "default", "created_at": 2.0}, + ]) + assert models._profile_has_user_projects() is True + + +def test_ensure_cron_project_default_create_true_ignores_gate(tmp_path): + """A direct call with no `create` argument must keep today's + unconditional-create behavior, even with zero user projects — only the + two gated callers opt in.""" + import api.models as models + + projects_file = tmp_path / "projects.json" + pid = models.ensure_cron_project() + + saved = json.loads(projects_file.read_text()) + assert any(p["project_id"] == pid and p["name"] == "Cron Jobs" for p in saved)