Skip to content

Commit 63e4506

Browse files
hassancs91claude
andcommitted
Stabilization: reconciled runs notify, pip tasks hardened, QA-sweep fixes
Reliability (roadmap 1.1 + 1.2, meant for 1.16 but never shipped): - Run.reconcile_stale now sends the script's normal failure notification for every run it flips to FAILED (running-past-deadline and lost-from-queue), best-effort per run so a broken channel can't stall the heartbeat. - Package operations: pip runs through EnvironmentService._run_pip (own process group + kill_process_tree on timeout, stdin=DEVNULL, PIP_NO_INPUT); each django-q task gets pip's cap + 60s so pip always dies first; the worker retry floor accounts for the longest pip task; the heartbeat reconciles stale PackageOperations; STALE_AFTER 15 -> 30 min. July QA sweep (docs/TEST_PLAN_e2e.md section 100): - F2: a plugins/<slug> folder with no Plugin row is moved aside (<slug>.orphaned-<ts>) instead of blocking every upload of that slug; a dev-mode folder is refused with a clear message. - F3: partial venvs are removed when `python -m venv` fails, and an unreferenced leftover folder is reclaimed on the next create. - F4: the interpreter picker probes `import ensurepip, venv` and hides interpreters that can't build a venv. - F5: pyrunner_db.connect() no longer claims psycopg ships with the runtime; it points at Environments -> Packages. - F6: plugin.json min_pyrunner is enforced at upload and activation. - F8: saving a changed admin URL slug warns that it applies after a restart. - F1: run-local-postgres.ps1 -Fresh removes the three named volumes explicitly after `down -v` and fails loudly if any survive. - F9: not reproducible - ScheduleFirstTickTests drives django-q's real scheduler loop and locks "interval fires once, clock modes don't fire on creation". Tests: core/test_stabilization_v1_17.py (27 tests); full core suite 1644 OK. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent a23bb51 commit 63e4506

12 files changed

Lines changed: 849 additions & 46 deletions

CHANGELOG.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,50 @@ begins tracking at the current release; earlier history is in the git log.
4141
files under the test runner's `DEBUG=False`), and an unpinned Ruff picked
4242
up 0.16's much larger default rule set. Both pinned down; no product code
4343
changed.
44+
- **A worker dying mid-run now triggers the run's failure notification**
45+
runs the stale-run reconciler flipped to *failed* never went through the
46+
normal finalize path, so the one failure the platform most needs to report
47+
(your worker was killed or restarted while your script ran) was the one it
48+
stayed silent about. Reconciled runs, including runs lost from the queue,
49+
now send the script's configured notifications.
50+
- **Bulk requirements install no longer gets stuck** — the pip subprocess and
51+
the worker task that hosts it both had a 600 s limit, so on a slow install
52+
the worker killed the task first, pip kept running orphaned in the venv,
53+
and the queue re-delivered the same install into it. Every package task
54+
now gets its own timeout above pip's cap, pip runs in its own process group
55+
(a timeout kills its build helpers too), with stdin closed and
56+
`PIP_NO_INPUT` set so it can never wait on a prompt; the worker retry
57+
window accounts for the longest pip task, and stuck package operations
58+
are reconciled by the worker heartbeat instead of only when someone opens
59+
the packages page.
60+
- **A failed environment create no longer poisons its name** — a `python -m
61+
venv` that failed halfway (typically Debian's `/usr/bin/python3` without
62+
`python3-venv`) left a folder behind that blocked the name forever with
63+
"Path already exists" and no way to clean up. Partial venvs are removed on
64+
failure, an existing folder no environment references is reclaimed, and the
65+
interpreter picker only lists Pythons that can actually bootstrap pip.
66+
- **A plugin's `min_pyrunner` is now enforced** — the manifest field was
67+
documented but never checked, so a plugin built for a newer PyRunner
68+
uploaded and activated on an older one and failed later in confusing ways.
69+
Upload and activation now refuse it with the required version.
70+
- **A stray plugin folder no longer blocks uploads of that slug** — a folder
71+
under the plugins directory with no plugin record (a delete that crashed
72+
halfway, or files shipped inside an image) made every upload of the same
73+
slug fail with "already exists" and nothing in the UI could remove it. It
74+
is now moved aside (never deleted) and the upload proceeds; a folder loaded
75+
in dev mode is left alone with a clear message.
76+
- **`pyrunner_db.connect()` tells the truth about `psycopg`** — the error
77+
claimed the driver "ships with PyRunner's runtime"; it doesn't ship inside
78+
script environments. The message now points at Environments → Packages.
79+
- **Changing the Django admin URL slug says it needs a restart** — routes are
80+
built at startup, so the old slug stayed live and the new one 404'd until
81+
the next restart, with nothing telling you so. Saving a changed slug now
82+
shows exactly that.
83+
- **`run-local-postgres.ps1 -Fresh` really wipes**`docker compose down -v`
84+
only removes volumes it can attribute to its own project, so a stack that
85+
had been brought up under another project name kept its month-old Postgres
86+
data and the "fresh" boot silently reused it. The launcher now removes the
87+
three named volumes explicitly and refuses to continue if any survive.
4488

4589
## [1.16.0] — July 29, 2026
4690

core/models/package.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ class Status(models.TextChoices):
8282
# An operation should never legitimately stay pending/running this long.
8383
# Past this, it almost certainly belongs to a worker that crashed/restarted
8484
# mid-task or a task stuck in a django-q2 re-queue loop.
85-
STALE_AFTER = datetime.timedelta(minutes=15)
85+
# Must outlast the longest package task (bulk install: 600s pip cap + 60s
86+
# worker grace) plus realistic queue wait, or a slow-but-alive install is
87+
# mislabelled failed while pip is still working.
88+
STALE_AFTER = datetime.timedelta(minutes=30)
8689

8790
class Meta:
8891
db_table = "package_operations"

core/models/run.py

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import datetime
6+
import logging
67
import uuid
78

89
from django.conf import settings
@@ -12,6 +13,9 @@
1213
from .workspace import WorkspaceScopedManager
1314

1415

16+
logger = logging.getLogger(__name__)
17+
18+
1519
class Run(models.Model):
1620
"""
1721
Represents a single execution of a script.
@@ -192,6 +196,7 @@ def reconcile_stale(cls) -> int:
192196

193197
now = timezone.now()
194198
reconciled = 0
199+
notify_pks: list = []
195200

196201
# RUNNING: dead once well past started_at + the script's own timeout +
197202
# grace. The deadline is per-run, and the candidate set is tiny by
@@ -220,7 +225,7 @@ def reconcile_stale(cls) -> int:
220225
f"{run.script.timeout_seconds}s timeout. A detached script "
221226
"process may have kept running after the worker died.]"
222227
)
223-
reconciled += cls.objects.filter(
228+
flipped = cls.objects.filter(
224229
pk=run.pk, status=cls.Status.RUNNING
225230
).update(
226231
status=cls.Status.FAILED,
@@ -229,14 +234,22 @@ def reconcile_stale(cls) -> int:
229234
ended_at=now,
230235
pid=None,
231236
)
237+
reconciled += flipped
238+
if flipped:
239+
notify_pks.append(run.pk)
232240

233241
# PENDING: only reconciled while workers are alive — with the cluster
234242
# down, "queued" is still the truth and these runs will execute when it
235243
# returns.
236244
if GlobalSettings.get_settings().worker_is_alive():
237-
reconciled += cls.objects.filter(
238-
status=cls.Status.PENDING,
239-
created_at__lt=now - cls.RECONCILE_PENDING_AFTER,
245+
stale_pending = list(
246+
cls.objects.filter(
247+
status=cls.Status.PENDING,
248+
created_at__lt=now - cls.RECONCILE_PENDING_AFTER,
249+
).values_list("pk", flat=True)
250+
)
251+
flipped = cls.objects.filter(
252+
pk__in=stale_pending, status=cls.Status.PENDING
240253
).update(
241254
status=cls.Status.FAILED,
242255
exit_code=-1,
@@ -248,9 +261,37 @@ def reconcile_stale(cls) -> int:
248261
),
249262
ended_at=now,
250263
)
251-
264+
reconciled += flipped
265+
if flipped:
266+
notify_pks.extend(
267+
cls.objects.filter(
268+
pk__in=stale_pending, status=cls.Status.FAILED
269+
).values_list("pk", flat=True)
270+
)
271+
272+
cls._notify_reconciled(notify_pks)
252273
return reconciled
253274

275+
@classmethod
276+
def _notify_reconciled(cls, pks) -> None:
277+
"""Send the normal failure notifications for reconciled runs.
278+
279+
A run the reconciler flips to FAILED never went through
280+
``execute_run_task``'s finalize path, so without this the one failure
281+
class the platform most needs to report - "your worker died mid-run" -
282+
was the one it stayed silent about. Best-effort per run: a broken
283+
channel must not stop the reconciler (which is the heartbeat).
284+
"""
285+
if not pks:
286+
return
287+
from core.services.notification_service import NotificationService
288+
289+
for run in cls.objects.filter(pk__in=pks).select_related("script"):
290+
try:
291+
NotificationService.send_notification(run)
292+
except Exception:
293+
logger.exception(f"Notification for reconciled run {run.pk} failed")
294+
254295
@property
255296
def duration(self) -> float | None:
256297
"""Return the duration in seconds, or None if not completed."""

core/script_helpers/pyrunner_db.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,10 @@ def connect(name: str, **kwargs):
139139
import psycopg
140140
except ImportError as e:
141141
raise PyRunnerDbError(
142-
"The 'psycopg' package is required for pyrunner_db.connect(). It "
143-
"ships with PyRunner's runtime; in a custom environment install it "
144-
"with: pip install psycopg[binary]"
142+
"The 'psycopg' package is not installed in this script's environment. "
143+
"Add 'psycopg[binary]' under Environments -> (this environment) -> "
144+
"Packages, then re-run. (pyrunner_db.dsn() / sqlalchemy_url() work "
145+
"without it if you bring your own driver.)"
145146
) from e
146147

147148
return psycopg.connect(dsn(name), **kwargs)

core/services/environment_service.py

Lines changed: 104 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,24 @@ def discover_python_versions(cls) -> list[dict]:
9494
pythons.append(p)
9595
seen_paths.add(p["path"])
9696

97-
return pythons
97+
# Debian/Ubuntu ship /usr/bin/python3 without ensurepip unless
98+
# python3-venv is installed; offering it produces a create that fails
99+
# with "ensurepip is not available" and an orphan folder. Probe first.
100+
return [p for p in pythons if cls._supports_venv(p["path"])]
101+
102+
@staticmethod
103+
def _supports_venv(python_path: str) -> bool:
104+
"""True when ``python -m venv`` can bootstrap pip with this interpreter."""
105+
try:
106+
result = subprocess.run(
107+
[python_path, "-c", "import ensurepip, venv"],
108+
capture_output=True,
109+
timeout=15,
110+
creationflags=subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0,
111+
)
112+
return result.returncode == 0
113+
except Exception:
114+
return False
98115

99116
@classmethod
100117
def _discover_via_py_launcher(cls) -> list[dict]:
@@ -235,9 +252,21 @@ def create_environment(
235252
except ValueError as e:
236253
return False, str(e)
237254

238-
# Check if path already exists
255+
# Check if path already exists. A folder no Environment row references is
256+
# the leftover of an earlier create that failed after ``python -m venv``
257+
# started (e.g. an interpreter without ensurepip): reclaim it instead of
258+
# blocking the name forever with no UI way out.
239259
if os.path.exists(full_path):
240-
return False, f"Path already exists: {full_path}"
260+
from core.models import Environment
261+
262+
if Environment.objects.filter(path=env_path).exists():
263+
return False, f"Path already exists: {full_path}"
264+
logger.warning(
265+
f"Reclaiming orphan environment folder {full_path} (no Environment row)"
266+
)
267+
shutil.rmtree(full_path, ignore_errors=True)
268+
if os.path.exists(full_path):
269+
return False, f"Path already exists and could not be removed: {full_path}"
241270

242271
# Ensure parent directory exists
243272
os.makedirs(os.path.dirname(full_path), exist_ok=True)
@@ -256,6 +285,7 @@ def create_environment(
256285

257286
if result.returncode != 0:
258287
error_msg = result.stderr or result.stdout or "Unknown error"
288+
cls._discard_partial_venv(full_path)
259289
return False, f"Failed to create venv: {error_msg}"
260290

261291
# Verify the environment was created
@@ -266,10 +296,18 @@ def create_environment(
266296
return True, "Environment created successfully"
267297

268298
except subprocess.TimeoutExpired:
299+
cls._discard_partial_venv(full_path)
269300
return False, "Timeout creating environment"
270301
except Exception as e:
302+
cls._discard_partial_venv(full_path)
271303
return False, f"Error creating environment: {str(e)}"
272304

305+
@staticmethod
306+
def _discard_partial_venv(full_path: str) -> None:
307+
"""Remove whatever a failed ``python -m venv`` left behind."""
308+
if os.path.exists(full_path):
309+
shutil.rmtree(full_path, ignore_errors=True)
310+
273311
@classmethod
274312
def delete_environment(cls, environment) -> tuple[bool, str]:
275313
"""
@@ -371,6 +409,66 @@ def pip_freeze(cls, environment) -> str:
371409
logger.error(f"pip freeze failed: {e}")
372410
return ""
373411

412+
# Wall-clock caps for the pip subprocess per operation. The django-q task
413+
# that hosts each one is given this + TASK_TIMEOUT_GRACE (see
414+
# ``task_timeout``) so pip's own timeout always fires first and kills the
415+
# tree; the worker-level timeout is only a backstop. Before this the two
416+
# were equal (600s), so the worker killed the task mid-install, the pip
417+
# process kept running orphaned, and the broker re-delivered the task
418+
# into the same venv - the "bulk install stuck" report.
419+
PIP_TIMEOUTS = {"install": 300, "uninstall": 120, "bulk_install": 600}
420+
TASK_TIMEOUT_GRACE = 60
421+
422+
@classmethod
423+
def task_timeout(cls, operation: str) -> int:
424+
"""django-q per-task timeout for a package operation (pip cap + grace)."""
425+
key = {"install": "install", "uninstall": "uninstall", "bulk_install": "bulk_install"}[
426+
str(operation)
427+
]
428+
return cls.PIP_TIMEOUTS[key] + cls.TASK_TIMEOUT_GRACE
429+
430+
@classmethod
431+
def max_task_timeout(cls) -> int:
432+
"""The longest package-operation task the cluster can host."""
433+
return max(cls.PIP_TIMEOUTS.values()) + cls.TASK_TIMEOUT_GRACE
434+
435+
@classmethod
436+
def _run_pip(cls, cmd: list, timeout: int) -> subprocess.CompletedProcess:
437+
"""Run a pip command with the same hardening as script runs.
438+
439+
- Own process group / session, so a timeout kills pip *and* the build
440+
backends it spawned (``kill_process_tree``), not just the leader.
441+
- ``stdin`` closed + ``PIP_NO_INPUT=1``: pip can never block on a
442+
prompt (e.g. a private index asking for credentials) inside a worker.
443+
Raises ``subprocess.TimeoutExpired`` after killing the tree.
444+
"""
445+
from core.executor_backends.local import kill_process_tree
446+
447+
popen_kwargs: dict = {
448+
"stdin": subprocess.DEVNULL,
449+
"stdout": subprocess.PIPE,
450+
"stderr": subprocess.PIPE,
451+
"text": True,
452+
"env": {**os.environ, "PIP_NO_INPUT": "1"},
453+
}
454+
if os.name == "nt":
455+
popen_kwargs["creationflags"] = (
456+
subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
457+
)
458+
else:
459+
popen_kwargs["start_new_session"] = True
460+
proc = subprocess.Popen(cmd, **popen_kwargs)
461+
try:
462+
stdout, stderr = proc.communicate(timeout=timeout)
463+
except subprocess.TimeoutExpired:
464+
kill_process_tree(proc.pid)
465+
try:
466+
proc.communicate(timeout=5)
467+
except Exception:
468+
pass
469+
raise
470+
return subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr)
471+
374472
@classmethod
375473
def install_package(
376474
cls, environment, package_spec: str
@@ -395,15 +493,7 @@ def install_package(
395493

396494
try:
397495
cmd = [pip_path, "install", package_spec]
398-
creationflags = subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0
399-
400-
result = subprocess.run(
401-
cmd,
402-
capture_output=True,
403-
text=True,
404-
timeout=300, # 5 minutes timeout
405-
creationflags=creationflags,
406-
)
496+
result = cls._run_pip(cmd, timeout=cls.PIP_TIMEOUTS["install"])
407497

408498
success = result.returncode == 0
409499
if success:
@@ -442,15 +532,7 @@ def uninstall_package(
442532

443533
try:
444534
cmd = [pip_path, "uninstall", "-y", package_name]
445-
creationflags = subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0
446-
447-
result = subprocess.run(
448-
cmd,
449-
capture_output=True,
450-
text=True,
451-
timeout=120,
452-
creationflags=creationflags,
453-
)
535+
result = cls._run_pip(cmd, timeout=cls.PIP_TIMEOUTS["uninstall"])
454536

455537
success = result.returncode == 0
456538
if success:
@@ -512,15 +594,7 @@ def install_requirements(
512594
temp_path = f.name
513595

514596
cmd = [pip_path, "install", "-r", temp_path]
515-
creationflags = subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0
516-
517-
result = subprocess.run(
518-
cmd,
519-
capture_output=True,
520-
text=True,
521-
timeout=600, # 10 minutes for bulk install
522-
creationflags=creationflags,
523-
)
597+
result = cls._run_pip(cmd, timeout=cls.PIP_TIMEOUTS["bulk_install"])
524598

525599
success = result.returncode == 0
526600
if success:

0 commit comments

Comments
 (0)