From a3c23bb5aba83a3fc1c6dfe39d241271cb97c04d Mon Sep 17 00:00:00 2001
From: fxd-jason <wujiachen7@gmail.com>
Date: Thu, 30 Apr 2026 16:46:48 +0800
Subject: [PATCH] fix(cron): import run_job inside _run_cron_tracked to fix
 NameError (#1312)

_run_cron_tracked() runs inside a worker thread (threading.Thread), so it
cannot see the caller's local import of run_job from _handle_cron_run.
Moved the import into _run_cron_tracked and removed the now-redundant
import from the route handler.

Fixes #1312, Fixes #1310
---
 api/routes.py                     |  2 +-
 tests/test_cron_run_job_import.py | 92 +++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_cron_run_job_import.py

diff --git a/api/routes.py b/api/routes.py
index 8524c3e2125..7ec990f5008 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -94,6 +94,7 @@ def _cron_output_content_window(text: str, limit: int = _CRON_OUTPUT_CONTENT_LIM
 
 def _run_cron_tracked(job):
     """Wrapper that tracks running state around cron.scheduler.run_job."""
+    from cron.scheduler import run_job  # import here — runs inside a worker thread
     try:
         run_job(job)
     finally:
@@ -3500,7 +3501,6 @@ def _handle_cron_run(handler, body):
     if not job_id:
         return bad(handler, "job_id required")
     from cron.jobs import get_job
-    from cron.scheduler import run_job
 
     job = get_job(job_id)
     if not job:
diff --git a/tests/test_cron_run_job_import.py b/tests/test_cron_run_job_import.py
new file mode 100644
index 00000000000..d0533fa376b
--- /dev/null
+++ b/tests/test_cron_run_job_import.py
@@ -0,0 +1,92 @@
+"""Regression test for #1312 / #1310 — _run_cron_tracked must import run_job.
+
+The function runs inside a worker thread (threading.Thread), so any
+names it references must be resolvable from that thread's scope.
+Before the fix, run_job was only imported inside _handle_cron_run
+(a local scope invisible to _run_cron_tracked), causing NameError.
+"""
+import ast
+import inspect
+from pathlib import Path
+
+import pytest
+
+ROUTES_PY = Path(__file__).resolve().parent.parent / "api" / "routes.py"
+
+
+def _get_function_source(func_name: str) -> str:
+    """Extract a top-level function's source via AST for stability."""
+    tree = ast.parse(ROUTES_PY.read_text(encoding="utf-8"))
+    for node in ast.iter_child_nodes(tree):
+        if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == func_name:
+            lines = ROUTES_PY.read_text(encoding="utf-8").splitlines()
+            return "\n".join(lines[node.lineno - 1 : node.end_lineno])
+    pytest.fail(f"Function {func_name} not found in {ROUTES_PY}")
+
+
+class TestRunCronTrackedImport:
+    """_run_cron_tracked must be self-contained — it runs in a worker thread."""
+
+    def test_run_job_imported_inside_function(self):
+        """run_job must be imported inside _run_cron_tracked, not relied on
+        from a caller's local scope."""
+        src = _get_function_source("_run_cron_tracked")
+        tree = ast.parse(src)
+        names_used = set()
+
+        class NameCollector(ast.NodeVisitor):
+            def visit_Name(self, node):
+                names_used.add(node.id)
+
+        ImportCollector = type(
+            "ImportCollector",
+            (ast.NodeVisitor,),
+            {
+                "imports": set(),
+                "visit_ImportFrom": lambda self, node: (
+                    self.imports.add(a.name for a in node.names),
+                ),
+            },
+        )
+
+        # Collect all names referenced in the function body
+        for node in ast.walk(tree):
+            if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load):
+                names_used.add(node.id)
+
+        # Collect imports inside the function
+        func_imports = set()
+        for node in ast.walk(tree):
+            if isinstance(node, ast.ImportFrom):
+                for alias in node.names:
+                    func_imports.add(alias.name if alias.asname is None else alias.asname)
+            elif isinstance(node, ast.Import):
+                for alias in node.names:
+                    func_imports.add(alias.name if alias.asname is None else alias.asname)
+
+        # run_job is referenced → must be imported inside the function
+        if "run_job" in names_used:
+            assert "run_job" in func_imports, (
+                "_run_cron_tracked references run_job but does not import it locally. "
+                "It runs in a worker thread and cannot rely on caller's local imports."
+            )
+
+    def test_handle_cron_run_does_not_import_run_job(self):
+        """After the fix, _handle_cron_run should NOT need to import run_job
+        itself — it's now _run_cron_tracked's responsibility."""
+        src = _get_function_source("_handle_cron_run")
+        tree = ast.parse(src)
+        imported_names = set()
+        for node in ast.walk(tree):
+            if isinstance(node, ast.ImportFrom):
+                for alias in node.names:
+                    imported_names.add(alias.name if alias.asname is None else alias.asname)
+        assert "run_job" not in imported_names, (
+            "_handle_cron_run still imports run_job — it should be moved to "
+            "_run_cron_tracked to avoid the NameError in worker threads."
+        )
+
+    def test_run_cron_tracked_calls_run_job(self):
+        """Sanity: the function still actually calls run_job."""
+        src = _get_function_source("_run_cron_tracked")
+        assert "run_job" in src, "_run_cron_tracked should call run_job"