From 67e527fd74f92dff4084418d4873899512d9d27a Mon Sep 17 00:00:00 2001 From: Jaret Bottoms <25535214+jbbottoms@users.noreply.github.com> Date: Sat, 11 Jul 2026 21:57:40 -0500 Subject: [PATCH] fix(cron): guard profile-switch polling races --- api/routes.py | 13 +- static/panels.js | 4 + tests/test_issue3460_cron_session_unread.py | 1 + ...est_issue5960_cron_unread_profile_scope.py | 133 +++++++++--------- 4 files changed, 75 insertions(+), 76 deletions(-) diff --git a/api/routes.py b/api/routes.py index 868217ed..999b8caa 100644 --- a/api/routes.py +++ b/api/routes.py @@ -19902,19 +19902,8 @@ def _handle_cron_recent(handler, parsed): since = 0.0 try: from cron.jobs import list_jobs - from api.profiles import ( - cron_profile_context_for_home, - get_active_profile_name, - get_hermes_home_for_profile, - ) - active_profile = get_active_profile_name() or "default" - active_home = get_hermes_home_for_profile(active_profile) - # Match the default Tasks list scope explicitly. The recent-completion - # endpoint feeds an actionable unread badge, so returning jobs from a - # different profile can strand an ID that has no visible row to open. - with cron_profile_context_for_home(active_home): - jobs = list_jobs(include_disabled=True) + jobs = list_jobs(include_disabled=True) completions = [] for job in jobs: job_id = str(job.get("id", "") or "") diff --git a/static/panels.js b/static/panels.js index 99453376..ee70350c 100644 --- a/static/panels.js +++ b/static/panels.js @@ -12618,9 +12618,11 @@ async function disableAuth(){ let _cronPollSince=Date.now()/1000; // track from page load let _cronPollTimer=null; let _cronUnreadCount=0; +let _cronPollGeneration=0; const _cronNewJobIds=new Set(); // track which job IDs had new completions (unread) function _resetCronUnreadForProfileSwitch(){ + _cronPollGeneration++; _cronNewJobIds.clear(); _cronPollSince=Date.now()/1000; updateCronBadge(); @@ -12637,7 +12639,9 @@ function startCronPolling(){ _cronPollTimer=setInterval(async()=>{ if(document.hidden) return; // don't poll when tab is in background try{ + const pollGeneration=_cronPollGeneration; const data=await api(`/api/crons/recent?since=${_cronPollSince}`); + if(pollGeneration!==_cronPollGeneration) return; if(data.completions&&data.completions.length>0){ for(const c of data.completions){ if(c.toast_notifications !== false){ diff --git a/tests/test_issue3460_cron_session_unread.py b/tests/test_issue3460_cron_session_unread.py index 47bd99fa..4a2ccbb3 100644 --- a/tests/test_issue3460_cron_session_unread.py +++ b/tests/test_issue3460_cron_session_unread.py @@ -486,6 +486,7 @@ function extractFunc(name) {{ let _cronPollSince = 10; let _cronPollTimer = null; let _cronUnreadCount = 0; +let _cronPollGeneration = 0; const _cronNewJobIds = new Set(); const markCalls = []; const toastCalls = []; diff --git a/tests/test_issue5960_cron_unread_profile_scope.py b/tests/test_issue5960_cron_unread_profile_scope.py index 054a30c4..e21f26c0 100644 --- a/tests/test_issue5960_cron_unread_profile_scope.py +++ b/tests/test_issue5960_cron_unread_profile_scope.py @@ -2,81 +2,45 @@ from __future__ import annotations -import io import json -import sys -import types -from contextlib import contextmanager +import shutil +import subprocess from pathlib import Path -from types import SimpleNamespace + +import pytest ROOT = Path(__file__).resolve().parents[1] PANELS_JS = (ROOT / "static" / "panels.js").read_text(encoding="utf-8") +ROUTES_PY = (ROOT / "api" / "routes.py").read_text(encoding="utf-8") +NODE = shutil.which("node") -class _JSONHandler: - def __init__(self): - self.status = None - self.response_headers = [] - self.wfile = io.BytesIO() - - def send_response(self, status): - self.status = status - - def send_header(self, key, value): - self.response_headers.append((key, value)) - - def end_headers(self): - pass +def _extract_function(source: str, name: str) -> str: + start = source.index(f"function {name}(") + brace = source.index("{", start) + depth = 1 + pos = brace + 1 + while depth and pos < len(source): + if source[pos] == "{": + depth += 1 + elif source[pos] == "}": + depth -= 1 + pos += 1 + assert depth == 0 + return source[start:pos] -def _payload(handler): - return json.loads(handler.wfile.getvalue().decode("utf-8")) +def test_recent_handler_reuses_dispatcher_cron_context_without_nesting(): + dispatch_start = ROUTES_PY.index('if parsed.path == "/api/crons/recent":') + dispatch_end = ROUTES_PY.index('if parsed.path == "/api/crons/status":', dispatch_start) + dispatch = ROUTES_PY[dispatch_start:dispatch_end] + handler_start = ROUTES_PY.index("def _handle_cron_recent(") + handler_end = ROUTES_PY.index("\ndef ", handler_start + 1) + handler = ROUTES_PY[handler_start:handler_end] - -def test_recent_completions_are_read_from_the_active_profile_home(monkeypatch): - import api.profiles as profiles - import api.routes as routes - - entered_homes = [] - - @contextmanager - def profile_context(home): - entered_homes.append(home) - yield - - cron_pkg = types.ModuleType("cron") - cron_pkg.__path__ = [] - cron_jobs = types.ModuleType("cron.jobs") - - def list_jobs(include_disabled=True): - assert entered_homes == ["/profiles/vops"] - return [ - { - "id": "vops-job", - "name": "Vops job", - "last_run_at": 20, - "last_status": "success", - } - ] - - cron_jobs.list_jobs = list_jobs - monkeypatch.setitem(sys.modules, "cron", cron_pkg) - monkeypatch.setitem(sys.modules, "cron.jobs", cron_jobs) - monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "vops") - monkeypatch.setattr( - profiles, "get_hermes_home_for_profile", lambda name: f"/profiles/{name}" - ) - monkeypatch.setattr(profiles, "cron_profile_context_for_home", profile_context) - monkeypatch.setattr(routes, "_latest_cron_session_info_for_jobs", lambda *args: {}) - - handler = _JSONHandler() - routes._handle_cron_recent(handler, SimpleNamespace(query="since=10")) - - assert handler.status == 200 - assert entered_homes == ["/profiles/vops"] - assert [row["job_id"] for row in _payload(handler)["completions"]] == ["vops-job"] + assert "with cron_profile_context():" in dispatch + assert "cron_profile_context_for_home" not in handler def test_successful_profile_switch_resets_unread_cron_state(): @@ -91,6 +55,47 @@ def test_successful_profile_switch_resets_unread_cron_state(): reset_start = PANELS_JS.index("function _resetCronUnreadForProfileSwitch(){") reset_end = PANELS_JS.index("\n}", reset_start) reset_body = PANELS_JS[reset_start:reset_end] + assert "_cronPollGeneration++;" in reset_body assert "_cronNewJobIds.clear();" in reset_body assert "_cronPollSince=Date.now()/1000;" in reset_body assert "updateCronBadge();" in reset_body + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_poll_started_before_switch_cannot_recreate_unread_state(): + polling = _extract_function(PANELS_JS, "startCronPolling") + reset = _extract_function(PANELS_JS, "_resetCronUnreadForProfileSwitch") + script = f""" +let _cronPollSince=10; +let _cronPollTimer=null; +let _cronUnreadCount=0; +let _cronPollGeneration=0; +const _cronNewJobIds=new Set(); +let intervalCallback=null; +let resolveApi; +global.document={{hidden:false}}; +global.setInterval=(callback)=>{{ intervalCallback=callback; return 1; }}; +global.api=()=>new Promise(resolve=>{{ resolveApi=resolve; }}); +global.showToast=()=>{{}}; +global.t=(key)=>key; +global.updateCronBadge=()=>{{ _cronUnreadCount=_cronNewJobIds.size; }}; +{polling} +{reset} +startCronPolling(); +(async()=>{{ + const stalePoll=intervalCallback(); + _resetCronUnreadForProfileSwitch(); + resolveApi({{completions:[{{job_id:'old-profile-job',completed_at:20}}]}}); + await stalePoll; + process.stdout.write(JSON.stringify({{ + unread:Array.from(_cronNewJobIds), + count:_cronUnreadCount, + generation:_cronPollGeneration, + }})); +}})().catch(error=>{{ console.error(error); process.exit(1); }}); +""" + result = subprocess.run( + [NODE, "-e", script], check=True, capture_output=True, text=True, timeout=30 + ) + state = json.loads(result.stdout) + assert state == {"unread": [], "count": 0, "generation": 1}