From c51b57893637b1b0635e1355ec17c10eac50d53a Mon Sep 17 00:00:00 2001 From: Oleg Date: Sat, 25 Apr 2026 03:47:32 -0700 Subject: [PATCH 1/2] fix(admin): rebuild deps for system-python deployments + force_deps escape hatch The admin_update endpoint only ran `pip install` if `.venv/bin/pip` existed, which silently skipped dependency rebuilds on system-python deployments (Homebrew/Debian). Result: pyproject pin bumps merged to main but the live daemon kept running stale package versions. Fix: - Fall back to `sys.executable -m pip install --break-system-packages` when no in-tree venv is present (PEP 668 externally-managed envs). - Trigger rebuild when pyproject.toml or uv.lock changed in the pulled diff, OR when the new `force_deps=true` query param is set. - Surface `deps_error` in the response so silent failures stop being silent. - Wire `force_deps` through the `update_and_restart` MCP tool. Tests: added 3 cases covering force_deps URL wiring, branch+force_deps combo, and deps_error surfacing. All 172 pinky_self tool tests pass. Co-Authored-By: Claude Opus 4 --- src/pinky_daemon/api.py | 66 +++++++++++++++++++++++++--------- src/pinky_self/server.py | 15 +++++++- tests/test_pinky_self_tools.py | 56 +++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 17 deletions(-) diff --git a/src/pinky_daemon/api.py b/src/pinky_daemon/api.py index bb222cea..765ae83e 100644 --- a/src/pinky_daemon/api.py +++ b/src/pinky_daemon/api.py @@ -8239,7 +8239,12 @@ async def set_release_channel(channel: str = "stable"): # ── Admin: Update & Restart ─────────────────────────── @app.post("/admin/update") - async def admin_update(branch: str = "", dry_run: bool = False, force: bool = False): + async def admin_update( + branch: str = "", + dry_run: bool = False, + force: bool = False, + force_deps: bool = False, + ): """Pull latest code, rebuild if needed, and restart the daemon. The process manager (launchctl/systemd) must be installed for @@ -8254,9 +8259,15 @@ async def admin_update(branch: str = "", dry_run: bool = False, force: bool = Fa (`git checkout -- .`) before pulling, to recover from a dirty working tree (e.g. stale build artifacts blocking the update). Untracked files are preserved — no `git clean`. Ignored when dry_run=True. + + force_deps=True: reinstall dependencies even when git HEAD didn't + change. Use this when installed package versions have drifted from + pyproject.toml (e.g., manual pip install bypassed, or the daemon was + seeded from a stale image). """ import shutil import subprocess as sp + import sys if not branch: channel = os.environ.get("PINKYBOT_CHANNEL", "stable") @@ -8408,23 +8419,45 @@ async def admin_update(branch: str = "", dry_run: bool = False, force: bool = Fa except Exception: summary = "" - # Detect dependency changes + # Detect dependency changes — rebuild whenever pyproject.toml or uv.lock + # changed in the pull, or when force_deps=True is passed (escape hatch + # for installed-vs-pinned drift that git diff can't see). deps_rebuilt = False + deps_error = "" try: - changed = sp.check_output( - ["git", "diff", "--name-only", before_hash, after_hash, "--", "pyproject.toml"], - cwd=repo_dir, stderr=sp.DEVNULL, timeout=10, - ).decode().strip() - if changed: - venv_pip = str(Path(repo_dir) / ".venv" / "bin" / "pip") - if Path(venv_pip).exists(): - sp.check_output( - [venv_pip, "install", "-e", ".[all]", "--quiet"], - cwd=repo_dir, stderr=sp.STDOUT, timeout=120, - ) - deps_rebuilt = True - except Exception: - pass + if before_hash != after_hash: + changed = sp.check_output( + ["git", "diff", "--name-only", before_hash, after_hash, "--", + "pyproject.toml", "uv.lock"], + cwd=repo_dir, stderr=sp.DEVNULL, timeout=10, + ).decode().strip() + else: + changed = "" + + if changed or force_deps: + # 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. + venv_pip = Path(repo_dir) / ".venv" / "bin" / "pip" + if venv_pip.exists(): + pip_cmd = [str(venv_pip), "install", "-e", ".[all]", "--quiet"] + else: + # PEP 668: system pythons (Homebrew, Debian) mark themselves + # externally-managed. --break-system-packages lets us install + # into the same env the daemon imports from. + pip_cmd = [ + sys.executable, "-m", "pip", "install", + "-e", ".[all]", "--quiet", "--break-system-packages", + ] + sp.check_output(pip_cmd, cwd=repo_dir, stderr=sp.STDOUT, timeout=180) + deps_rebuilt = True + except sp.CalledProcessError as e: + deps_error = f"pip install failed: {e.output.decode()[:500] if e.output else e}" + _log(f"admin: {deps_error}") + except Exception as e: + deps_error = f"deps rebuild failed: {e}" + _log(f"admin: {deps_error}") # Always rebuild frontend on update to keep compiled assets fresh frontend_rebuilt = False @@ -8455,6 +8488,7 @@ async def admin_update(branch: str = "", dry_run: bool = False, force: bool = Fa "release": target_tag if use_release_tags else None, "commits": summary.splitlines() if summary else [], "deps_rebuilt": deps_rebuilt, + "deps_error": deps_error or None, "frontend_rebuilt": frontend_rebuilt, "frontend_error": frontend_error or None, "forced_reset": forced_reset, diff --git a/src/pinky_self/server.py b/src/pinky_self/server.py index e34c8df8..897465aa 100644 --- a/src/pinky_self/server.py +++ b/src/pinky_self/server.py @@ -1933,12 +1933,21 @@ def check_for_updates() -> str: return "\n".join(parts) @mcp.tool() - def update_and_restart(branch: str = "", force: bool = False) -> str: + def update_and_restart( + branch: str = "", + force: bool = False, + force_deps: bool = False, + ) -> str: """Pull latest code, rebuild if needed, and restart the daemon. force=True discards local mods to TRACKED files before pulling. Use to recover from a dirty working tree (e.g. stale build artifacts). Untracked files are preserved. + + force_deps=True reinstalls dependencies even when git HEAD didn't + move. Useful when installed package versions have drifted from + pyproject.toml (e.g., daemon seeded from a stale image, or a + previous deploy skipped the pip install step). """ url = "/admin/update" params = [] @@ -1946,6 +1955,8 @@ def update_and_restart(branch: str = "", force: bool = False) -> str: params.append(f"branch={branch}") if force: params.append("force=true") + if force_deps: + params.append("force_deps=true") if params: url += "?" + "&".join(params) result = _api("POST", url) @@ -1977,6 +1988,8 @@ def update_and_restart(branch: str = "", force: bool = False) -> str: if result.get("deps_rebuilt"): parts.append("\nDependencies rebuilt.") + if result.get("deps_error"): + parts.append(f"\nDeps rebuild error: {result['deps_error']}") if result.get("frontend_rebuilt"): parts.append("Frontend rebuilt.") diff --git a/tests/test_pinky_self_tools.py b/tests/test_pinky_self_tools.py index 566b0875..86d41c1d 100644 --- a/tests/test_pinky_self_tools.py +++ b/tests/test_pinky_self_tools.py @@ -1513,6 +1513,62 @@ def _urlopen(req, timeout=30): assert "force=true" in captured["url"] assert "branch=beta" in captured["url"] + def test_force_deps_in_url(self, srv): + """force_deps=True must reach /admin/update as a query param.""" + captured = {} + + def _urlopen(req, timeout=30): + captured["url"] = req.full_url if hasattr(req, "full_url") else str(req) + body = json.dumps({ + "updated": True, + "before_hash": "a", "after_hash": "a", + "deps_rebuilt": True, "restarting": True, + }).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = lambda s: s + resp.__exit__ = MagicMock(return_value=False) + return resp + + with patch("urllib.request.urlopen", side_effect=_urlopen): + _tools(srv)["update_and_restart"](force_deps=True) + assert "force_deps=true" in captured["url"] + + def test_force_deps_with_branch(self, srv): + """force_deps and branch must combine cleanly in the URL.""" + captured = {} + + def _urlopen(req, timeout=30): + captured["url"] = req.full_url if hasattr(req, "full_url") else str(req) + body = json.dumps({ + "updated": True, + "before_hash": "a", "after_hash": "a", + "restarting": False, + }).encode() + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = lambda s: s + resp.__exit__ = MagicMock(return_value=False) + return resp + + with patch("urllib.request.urlopen", side_effect=_urlopen): + _tools(srv)["update_and_restart"](branch="beta", force_deps=True) + assert "branch=beta" in captured["url"] + assert "force_deps=true" in captured["url"] + + def test_deps_error_surfaced(self, srv): + """Backend deps_error string is surfaced to the caller.""" + with _ok({ + "updated": True, + "before_hash": "a", "after_hash": "b", + "deps_rebuilt": False, + "deps_error": "pip install failed: boom", + "restarting": True, + }): + result = _tools(srv)["update_and_restart"]() + assert "deps rebuild error" in result.lower() + assert "boom" in result + # ── restart_daemon ──────────────────────────────────────────────────────────── From 65004cebfee664f1cbeda30d593aaf970404d4aa Mon Sep 17 00:00:00 2001 From: Oleg Date: Tue, 5 May 2026 20:08:14 -0700 Subject: [PATCH 2/2] test(admin): cover force_deps + force interaction at endpoint level Adds 4 tests in TestAdminUpdateForceDepsIntegration verifying: - force_deps=True triggers pip install on a clean (no-op) pull - force=True + force_deps=True combine correctly (reset + reinstall) - default behavior skips pip when pyproject.toml is unchanged - pip install failures surface as deps_error in the response Mirrors the matching MCP-tool-level coverage in test_pinky_self_tools.py. Co-Authored-By: Claude Opus 4.7 --- tests/test_admin_update.py | 108 +++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/test_admin_update.py b/tests/test_admin_update.py index d1595bfd..f34362a0 100644 --- a/tests/test_admin_update.py +++ b/tests/test_admin_update.py @@ -239,3 +239,111 @@ def fail_on_pull(cmd, **kwargs): body = r.json() assert "error" in body assert "git pull failed" in body["error"] + + +class TestAdminUpdateForceDepsIntegration: + """force_deps (PR #323) and force (PR #390) are orthogonal — verify they cooperate.""" + + def _track_pip_calls(self, gm): + """Wrap _GitMock so we also log any pip-install invocation.""" + gm.pip_calls: list[list[str]] = [] + original_call = gm.__call__ + + def wrapped(cmd, **kwargs): + cmd_list = list(cmd) + # Detect pip install: either `/path/to/pip install ...` or + # `python -m pip install ...`. The first arg is the executable. + is_pip = ( + "install" in cmd_list + and ( + cmd_list[0].endswith("/pip") + or cmd_list[0] == "pip" + or "pip" in cmd_list # for `python -m pip ...` form + ) + ) + if is_pip: + gm.pip_calls.append(cmd_list) + return b"" + return original_call(cmd, **kwargs) + + return wrapped + + 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") + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force_deps=true") + assert r.status_code == 200 + body = r.json() + assert body.get("deps_rebuilt") is True + assert body.get("deps_error") is None + assert len(gm.pip_calls) == 1, f"expected exactly 1 pip install call, got {gm.pip_calls}" + # Should target the editable install + assert "-e" in gm.pip_calls[0] + assert ".[all]" in gm.pip_calls[0] + + def test_force_and_force_deps_combine(self): + """Both flags together: dirty tree gets reset AND deps get rebuilt.""" + gm = _GitMock(dirty_files=["frontend-dist/index.html"]) + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta&force=true&force_deps=true") + assert r.status_code == 200 + body = r.json() + assert body.get("forced_reset") is True + assert body.get("forced_files") == ["frontend-dist/index.html"] + assert body.get("deps_rebuilt") is True + assert gm.did_force_reset() + assert len(gm.pip_calls) == 1 + + def test_no_force_deps_skips_pip_when_pyproject_unchanged(self): + """Default behavior: don't reinstall when pyproject.toml didn't change.""" + gm = _GitMock(dirty_files=[]) + wrapped = self._track_pip_calls(gm) + with ( + patch("subprocess.check_output", side_effect=wrapped), + patch("shutil.which", return_value=None), + patch("os.kill"), + ): + client = _make_client() + r = client.post("/admin/update?branch=beta") + body = r.json() + assert body.get("deps_rebuilt") is False + assert gm.pip_calls == [] + + def test_deps_error_surfaced_when_pip_fails(self): + """If pip install errors out, deps_error is populated and surfaced.""" + gm = _GitMock(dirty_files=[], before_hash="same1", after_hash="same1") + + def fail_on_pip(cmd, **kwargs): + cmd_list = list(cmd) + is_pip = ( + "install" in cmd_list + and (cmd_list[0].endswith("/pip") or cmd_list[0] == "pip" or "pip" in cmd_list) + ) + if is_pip: + 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() + r = client.post("/admin/update?branch=beta&force_deps=true") + body = r.json() + assert body.get("deps_rebuilt") is False + assert body.get("deps_error") + assert "pip install failed" in body["deps_error"]