fix: make stop wait for exit and report the real version

stackvox/cli.py

@@ -432,12 +432,21 @@ def _cmd_status(_: argparse.Namespace) -> int:
         rc = 0
         # The running daemon can lag the installed package if it wasn't restarted
         # after an upgrade — surface that skew instead of trusting it blindly.
+        # Only a daemon that is genuinely BEHIND wants restarting; a newer one
+        # means the client just run is the older install (a checkout alongside a
+        # global one), and restarting would downgrade what's serving.
         got, running_version = daemon.version()
         if got and running_version != installed:
-            print(
-                f"daemon running {running_version}, but {installed} is installed "
-                f"— restart the daemon (stackvox stop; stackvox serve) to pick it up"
-            )
+            if updates._is_newer(installed, running_version):
+                print(
+                    f"daemon running {running_version}, but {installed} is installed "
+                    f"— restart the daemon (stackvox stop; stackvox serve) to pick it up"
+                )
+            else:
+                print(
+                    f"daemon running {running_version} is newer than the {installed} client "
+                    f"you just ran; no restart needed"
+                )
     else:
         print("stopped")
         rc = 1

stackvox/daemon.py

@@ -26,6 +26,7 @@
 import socketserver
 import sys
 import threading
+import time
 from typing import Any
 
 import sounddevice as sd
@@ -42,6 +43,10 @@
 WORKER_POLL_SECONDS = 0.5
 CLIENT_TIMEOUT_SECONDS = 1.0
 PING_TIMEOUT_SECONDS = 0.5
+# How long `stop` waits for the daemon process to actually exit after it has
+# acknowledged the request, and how often it re-checks.
+STOP_TIMEOUT_SECONDS = 5.0
+STOP_POLL_SECONDS = 0.05
 RECV_BYTES = 1024
 
 
@@ -273,14 +278,19 @@ def _pid_alive(pid: int) -> bool:
         return False
 
 
-def is_running() -> bool:
+def _read_pid() -> int | None:
+    """The pid recorded in the pid file, or None if absent or unparseable."""
     if not PID_PATH.exists():
-        return False
+        return None
     try:
-        pid = int(PID_PATH.read_text().strip())
-    except ValueError:
-        return False
-    return _pid_alive(pid)
+        return int(PID_PATH.read_text().strip())
+    except (ValueError, OSError):
+        return None
+
+
+def is_running() -> bool:
+    pid = _read_pid()
+    return pid is not None and _pid_alive(pid)
 
 
 def _check_for_update_async() -> None:
@@ -359,8 +369,29 @@ def say(
     return send(req)
 
 
-def stop() -> tuple[bool, str]:
-    return send({"command": "stop"})
+def stop(timeout: float = STOP_TIMEOUT_SECONDS) -> tuple[bool, str]:
+    """Shut the daemon down and wait until the process has actually gone.
+
+    The daemon acknowledges the request and only then unwinds `serve_forever`
+    and removes its pid file, so returning on the ack alone made
+    `stackvox stop && stackvox serve` race: `serve` still saw a live pid, refused
+    to start, and the shutdown then completed, leaving nothing running at all.
+    Reporting success before the process has exited makes that sequence, the one
+    `status` recommends, silently useless.
+    """
+    pid = _read_pid()
+    ok, resp = send({"command": "stop"})
+    if not ok or pid is None:
+        # Nothing acknowledged, or no pid to watch: the caller's own
+        # `is_running` check is as good as it gets.
+        return ok, resp
+    deadline = time.monotonic() + timeout
+    while True:
+        if not _pid_alive(pid):
+            return True, resp
+        if time.monotonic() >= deadline:
+            return False, f"daemon acknowledged stop but pid {pid} is still alive after {timeout:g}s"
+        time.sleep(STOP_POLL_SECONDS)
 
 
 def cancel() -> tuple[bool, str]:

stackvox/updates.py

@@ -18,6 +18,7 @@
 import json
 import logging
 import os
+import sys
 import urllib.error
 import urllib.request
 from dataclasses import dataclass
@@ -26,14 +27,51 @@
 from importlib.metadata import version as _pkg_version
 from pathlib import Path
 
+if sys.version_info >= (3, 11):
+    import tomllib
+else:  # pragma: no cover - covered by 3.10 CI
+    import tomli as tomllib
+
 from stackvox.paths import cache_dir
 
 logger = logging.getLogger(__name__)
 
 
+def _source_tree_version(pyproject: Path | None = None) -> str | None:
+    """Our version from `pyproject.toml`, when running from a source checkout.
+
+    An editable install records its version in dist metadata at install time, so
+    the moment a release bumps `pyproject.toml` that metadata goes stale. The
+    developer then gets a phantom "update available: 0.9.0 -> 0.11.0" against
+    their own tree, and `status` reads the running daemon as newer than the
+    "installed" package and tells them to restart it for no reason.
+
+    Returns None when there's no such file (a normal pip/pipx install) or when it
+    isn't ours, so the metadata path stays authoritative for real installs.
+    """
+    path = pyproject or Path(__file__).resolve().parent.parent / "pyproject.toml"
+    try:
+        with path.open("rb") as handle:
+            data = tomllib.load(handle)
+    except (OSError, tomllib.TOMLDecodeError):
+        return None
+    project = data.get("project")
+    if not isinstance(project, dict) or project.get("name") != "stackvox":
+        return None
+    version = project.get("version")
+    return version if isinstance(version, str) else None
+
+
 def _current_version() -> str:
     """Read our own installed version. Late-bound so importing this module
-    early in the package init chain doesn't trip a circular import."""
+    early in the package init chain doesn't trip a circular import.
+
+    A source checkout wins over dist metadata, which goes stale on an editable
+    install after every release.
+    """
+    from_source = _source_tree_version()
+    if from_source is not None:
+        return from_source
     try:
         return _pkg_version("stackvox")
     except PackageNotFoundError:

tests/test_cli.py

@@ -292,6 +292,27 @@ def test_running_flags_daemon_version_skew(self, mocker, capsys):
         assert "0.5.0" in out and "0.8.0" in out  # the skew is surfaced
         assert "restart" in out.lower()
 
+    def test_running_does_not_advise_restart_when_the_daemon_is_ahead(self, mocker, capsys):
+        """A daemon NEWER than the client means the client is the older install,
+        so restarting would downgrade what's serving."""
+        mocker.patch.object(cli.daemon, "is_running", return_value=True)
+        pid_path = mocker.MagicMock()
+        pid_path.read_text.return_value = "1\n"
+        mocker.patch.object(cli.daemon, "PID_PATH", pid_path)
+        mocker.patch.object(cli.daemon, "SOCKET_PATH", "/tmp/x.sock")
+        mocker.patch.object(cli.daemon, "version", return_value=(True, "0.11.0"))
+        mocker.patch.object(cli.updates, "_current_version", return_value="0.9.0")
+        mocker.patch.object(cli.updates, "check_for_update", return_value=None)
+
+        rc = cli._cmd_status(_ns())
+
+        assert rc == 0
+        out = capsys.readouterr().out
+        assert "0.11.0" in out and "0.9.0" in out
+        assert "stackvox stop" not in out  # the restart advice itself
+        assert "newer" in out.lower()
+        assert "no restart needed" in out
+
     def test_stopped_returns_one(self, mocker, capsys):
         mocker.patch.object(cli.daemon, "is_running", return_value=False)
         rc = cli._cmd_status(_ns())

tests/test_daemon.py

@@ -78,6 +78,9 @@ def test_say_without_overrides_passes_only_text(self, mocker):
         assert send.call_args.args[0] == {"text": "hi"}
 
     def test_stop_sends_command_stop(self, mocker):
+        # _read_pid is mocked out so the wait-for-exit loop can't reach a real
+        # daemon's pid file and block on a live process.
+        mocker.patch.object(daemon, "_read_pid", return_value=None)
         send = mocker.patch.object(daemon, "send", return_value=(True, "ok"))
         daemon.stop()
         assert send.call_args.args[0] == {"command": "stop"}
@@ -136,3 +139,72 @@ def test_disabled_when_coreaudio_unavailable(self, mocker, caplog):
         with caplog.at_level(logging.DEBUG, logger="stackvox.daemon"):
             daemon._start_device_watcher()  # must not raise
         assert any("CoreAudio unavailable" in r.message for r in caplog.records)
+
+
+class TestStopWaitsForExit:
+    """`stop` reports success only once the process has actually gone.
+
+    The daemon acknowledges the request before unwinding, so returning on the
+    ack made `stackvox stop && stackvox serve` race: serve saw a live pid and
+    refused, then the shutdown finished, leaving nothing running.
+    """
+
+    def test_returns_send_failure_unchanged(self, mocker):
+        mocker.patch.object(daemon, "_read_pid", return_value=4242)
+        mocker.patch.object(daemon, "send", return_value=(False, "daemon not running"))
+
+        actual = daemon.stop()
+
+        assert actual == (False, "daemon not running")
+
+    def test_does_not_wait_when_no_pid_is_recorded(self, mocker):
+        mocker.patch.object(daemon, "_read_pid", return_value=None)
+        mocker.patch.object(daemon, "send", return_value=(True, "ok"))
+        alive = mocker.patch.object(daemon, "_pid_alive")
+
+        actual = daemon.stop()
+
+        assert actual == (True, "ok")
+        alive.assert_not_called()
+
+    def test_waits_until_the_process_has_exited(self, mocker):
+        mocker.patch.object(daemon, "_read_pid", return_value=4242)
+        mocker.patch.object(daemon, "send", return_value=(True, "ok"))
+        alive = mocker.patch.object(daemon, "_pid_alive", side_effect=[True, True, False])
+        sleep = mocker.patch.object(daemon.time, "sleep")
+
+        actual = daemon.stop()
+
+        assert actual == (True, "ok")
+        assert alive.call_count == 3
+        assert sleep.call_count == 2
+
+    def test_fails_when_the_process_outlives_the_timeout(self, mocker):
+        mocker.patch.object(daemon, "_read_pid", return_value=4242)
+        mocker.patch.object(daemon, "send", return_value=(True, "ok"))
+        mocker.patch.object(daemon, "_pid_alive", return_value=True)
+        mocker.patch.object(daemon.time, "sleep")
+
+        ok, resp = daemon.stop(timeout=0.0)
+
+        assert ok is False
+        assert "still alive" in resp
+        assert "4242" in resp
+
+
+class TestReadPid:
+    def test_returns_none_when_missing(self, mocker, tmp_path):
+        mocker.patch.object(daemon, "PID_PATH", tmp_path / "missing.pid")
+        assert daemon._read_pid() is None
+
+    def test_returns_none_when_unparseable(self, mocker, tmp_path):
+        pid = tmp_path / "garbage.pid"
+        pid.write_text("not-a-number")
+        mocker.patch.object(daemon, "PID_PATH", pid)
+        assert daemon._read_pid() is None
+
+    def test_returns_the_recorded_pid(self, mocker, tmp_path):
+        pid = tmp_path / "live.pid"
+        pid.write_text("4242\n")
+        mocker.patch.object(daemon, "PID_PATH", pid)
+        assert daemon._read_pid() == 4242

tests/test_updates.py

@@ -218,3 +218,48 @@ def test_includes_versions_and_upgrade_command(self):
         assert "0.3.1" in msg
         assert "0.4.0" in msg
         assert "pipx upgrade stackvox" in msg
+
+
+class TestSourceTreeVersion:
+    """An editable install's dist metadata goes stale the moment a release bumps
+    pyproject, so a source checkout has to win over it."""
+
+    def test_reads_the_version_from_our_own_pyproject(self, tmp_path):
+        path = tmp_path / "pyproject.toml"
+        path.write_text('[project]\nname = "stackvox"\nversion = "1.2.3"\n')
+        assert updates._source_tree_version(path) == "1.2.3"
+
+    def test_ignores_a_pyproject_that_is_not_ours(self, tmp_path):
+        path = tmp_path / "pyproject.toml"
+        path.write_text('[project]\nname = "something-else"\nversion = "1.2.3"\n')
+        assert updates._source_tree_version(path) is None
+
+    def test_returns_none_when_there_is_no_pyproject(self, tmp_path):
+        assert updates._source_tree_version(tmp_path / "nope.toml") is None
+
+    def test_returns_none_when_the_file_is_malformed(self, tmp_path):
+        path = tmp_path / "pyproject.toml"
+        path.write_text("not valid toml {{{")
+        assert updates._source_tree_version(path) is None
+
+    def test_returns_none_when_the_version_is_absent(self, tmp_path):
+        path = tmp_path / "pyproject.toml"
+        path.write_text('[project]\nname = "stackvox"\n')
+        assert updates._source_tree_version(path) is None
+
+
+class TestCurrentVersion:
+    def test_source_tree_beats_stale_dist_metadata(self, mocker):
+        mocker.patch.object(updates, "_source_tree_version", return_value="0.11.0")
+        mocker.patch.object(updates, "_pkg_version", return_value="0.9.0")
+        assert updates._current_version() == "0.11.0"
+
+    def test_falls_back_to_dist_metadata_for_a_real_install(self, mocker):
+        mocker.patch.object(updates, "_source_tree_version", return_value=None)
+        mocker.patch.object(updates, "_pkg_version", return_value="0.11.0")
+        assert updates._current_version() == "0.11.0"
+
+    def test_reports_unknown_when_not_installed_at_all(self, mocker):
+        mocker.patch.object(updates, "_source_tree_version", return_value=None)
+        mocker.patch.object(updates, "_pkg_version", side_effect=updates.PackageNotFoundError)
+        assert updates._current_version() == "0.0.0+unknown"