Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions src/pinky_daemon/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11836,14 +11836,42 @@ def _run_update() -> dict:
deps_drift = []

if changed or force_deps or deps_drift:
# Prefer project venv pip if present, else use the running daemon's
# interpreter (sys.executable). This works for both venv and
# system-python deployments — the prior `.venv/bin/pip`-only path
# silently skipped rebuilds on system-python hosts.
# Prefer a functional project venv pip, else use the running
# daemon's interpreter (sys.executable). A vestigial .venv can
# retain bin/pip even after pip disappears from its interpreter,
# so file existence alone is not enough to select it.
venv_pip = Path(repo_dir) / ".venv" / "bin" / "pip"
venv_python = Path(repo_dir) / ".venv" / "bin" / "python"
pip_cmd: list[str] | None = None
if venv_pip.exists():
pip_cmd = [str(venv_pip), "install", "-e", ".[all]", "--quiet"]
else:
try:
sp.check_output(
[str(venv_python), "-m", "pip", "--version"],
cwd=repo_dir, stderr=sp.STDOUT, timeout=10,
)
except Exception as probe_exc:
probe_output = getattr(probe_exc, "output", None)
if isinstance(probe_output, bytes):
probe_detail = probe_output.decode(errors="replace")
elif probe_output:
probe_detail = str(probe_output)
else:
probe_detail = f"{type(probe_exc).__name__}: {probe_exc}"
probe_detail = " ".join(probe_detail.split())[:500]
if not probe_detail:
probe_detail = f"{type(probe_exc).__name__}: {probe_exc}"
_log(
"admin: .venv exists but its pip is broken "
f"({probe_detail}) — treating it as vestigial and "
f"falling back to {sys.executable}"
)
else:
pip_cmd = [
str(venv_python), "-m", "pip", "install",
"-e", ".[all]", "--quiet",
]

if pip_cmd is None:
# PEP 668: system pythons (Homebrew, Debian) mark themselves
# externally-managed. --break-system-packages lets us install
# into the same env the daemon imports from.
Expand Down Expand Up @@ -11924,6 +11952,20 @@ def _run_update() -> dict:

result = await asyncio.to_thread(_run_update)

if result.get("deps_error"):
try:
delivered = await scheduler._owner_notify_callback(
"admin",
"ADMIN UPDATE DEPENDENCY REBUILD FAILED: "
f"{result['deps_error']}. The code update may have landed "
"without required dependencies; inspect the daemon log and "
"rerun the dependency rebuild.",
)
if not delivered:
_log("admin: dependency rebuild owner alert was not delivered")
except Exception as notify_exc:
_log(f"admin: dependency rebuild owner alert failed: {notify_exc}")

# Schedule graceful restart if anything changed
if result.get("restarting"):
import signal
Expand Down
114 changes: 113 additions & 1 deletion tests/test_admin_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@

import os
import subprocess as sp
import sys
import tempfile
from pathlib import Path
from unittest.mock import patch
from unittest.mock import AsyncMock, patch

import pytest
from fastapi.testclient import TestClient

from pinky_daemon.self_update import DeployDecision

_REAL_PATH_EXISTS = Path.exists


@pytest.fixture(autouse=True)
def _stub_resolve_and_verify():
Expand Down Expand Up @@ -513,6 +516,12 @@ def wrapped(cmd, **kwargs):

return wrapped

@staticmethod
def _with_venv_pip(path: Path) -> bool:
if str(path).endswith("/.venv/bin/pip"):
return True
return _REAL_PATH_EXISTS(path)

def test_force_deps_triggers_pip_install_even_on_clean_pull(self):
"""force_deps=True → pip install runs even when nothing changed in git."""
gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1")
Expand Down Expand Up @@ -600,6 +609,109 @@ def fail_on_pip(cmd, **kwargs):
assert body.get("deps_error")
assert "pip install failed" in body["deps_error"]

def test_broken_venv_pip_falls_back_and_logs_vestigial_warning(self):
gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1")
calls: list[list[str]] = []
logs: list[str] = []

def broken_venv_pip(cmd, **kwargs):
cmd_list = list(cmd)
calls.append(cmd_list)
if cmd_list[1:] == ["-m", "pip", "--version"]:
raise sp.CalledProcessError(
1, cmd, output=b".venv/bin/python: No module named pip",
)
if "install" in cmd_list and "pip" in cmd_list:
return b""
return gm(cmd, **kwargs)

with (
patch("subprocess.check_output", side_effect=broken_venv_pip),
patch("pathlib.Path.exists", autospec=True, side_effect=self._with_venv_pip),
patch("pinky_daemon.api._log", side_effect=logs.append),
patch("shutil.which", return_value=None),
patch("os.kill"),
):
client = _make_client()
r = client.post("/admin/update?branch=main&force_deps=true")

assert r.status_code == 200
assert r.json()["deps_rebuilt"] is True
assert any(
call[:4] == [sys.executable, "-m", "pip", "install"]
and "--break-system-packages" in call
for call in calls
)
install_calls = [call for call in calls if "install" in call and "pip" in call]
assert len(install_calls) == 1
assert "--break-system-packages" in install_calls[0]
warnings = [line for line in logs if "treating it as vestigial" in line]
assert len(warnings) == 1
assert "No module named pip" in warnings[0]
assert f"falling back to {sys.executable}" in warnings[0]

def test_functional_venv_pip_is_still_preferred(self):
gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1")
calls: list[list[str]] = []
project_venv_python: list[str] = []

def functional_venv_pip(cmd, **kwargs):
cmd_list = list(cmd)
calls.append(cmd_list)
if cmd_list[1:] == ["-m", "pip", "--version"]:
project_venv_python.append(cmd_list[0])
return b"pip 26.0"
if cmd_list[0].endswith("/.venv/bin/python") and "install" in cmd_list:
return b""
return gm(cmd, **kwargs)

with (
patch("subprocess.check_output", side_effect=functional_venv_pip),
patch("pathlib.Path.exists", autospec=True, side_effect=self._with_venv_pip),
patch("shutil.which", return_value=None),
patch("os.kill"),
):
client = _make_client()
r = client.post("/admin/update?branch=main&force_deps=true")

assert r.status_code == 200
assert r.json()["deps_rebuilt"] is True
assert any(call[1:] == ["-m", "pip", "--version"] for call in calls)
assert any(
call[0] == project_venv_python[0]
and call[1:4] == ["-m", "pip", "install"]
and "--break-system-packages" not in call
for call in calls
)
install_calls = [call for call in calls if "install" in call and "pip" in call]
assert len(install_calls) == 1
assert "--break-system-packages" not in install_calls[0]

def test_deps_error_alerts_owner_once(self):
gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1")

def fail_on_pip(cmd, **kwargs):
cmd_list = list(cmd)
if "install" in cmd_list and "pip" in cmd_list:
raise sp.CalledProcessError(1, cmd, output=b"ERROR: package not found")
return gm(cmd, **kwargs)

with (
patch("subprocess.check_output", side_effect=fail_on_pip),
patch("shutil.which", return_value=None),
patch("os.kill"),
):
client = _make_client()
owner_notify = AsyncMock(return_value=True)
client.app.state.scheduler._owner_notify_callback = owner_notify
r = client.post("/admin/update?branch=main&force_deps=true")

assert r.status_code == 200
assert r.json()["deps_error"]
owner_notify.assert_awaited_once()
assert owner_notify.await_args.args[0] == "admin"
assert "ADMIN UPDATE DEPENDENCY REBUILD FAILED" in owner_notify.await_args.args[1]


class TestInstalledDepsDriftDetection:
"""Direct unit tests for `_check_installed_deps_drift`.
Expand Down
Loading