fix(admin): rebuild deps for system-python (rebase of #323 + integration tests)

src/pinky_daemon/api.py

@@ -6654,7 +6654,7 @@
         if not adapter:
             raise HTTPException(503, f"No {platform} adapter for {agent_name_req}")
 
         # Resolve Giphy API key: settings > env > public fallback
         api_key = agents.get_setting("GIPHY_API_KEY") or os.environ.get("GIPHY_API_KEY", giphy_public_key)
 
         # Search Giphy
@@ -8239,7 +8239,12 @@
     # ── 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 @@
         (`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 @@
         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 @@
             "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,

src/pinky_self/server.py

@@ -1933,19 +1933,30 @@ 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 = []
             if branch:
                 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.")
 

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"]

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 ────────────────────────────────────────────────────────────