Make the parent-death watchdog work on Windows, and the test suite honest there

app/core/process.py

@@ -32,12 +32,33 @@ def process_exists(pid: int) -> bool:
 
     PROCESS_QUERY_LIMITED_INFORMATION = 0x1000
     ERROR_INVALID_PARAMETER = 87
+    STILL_ACTIVE = 259
     kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
     handle = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, False, pid)
-    if handle:
+    if not handle:
+        return ctypes.get_last_error() != ERROR_INVALID_PARAMETER
+
+    # Opening it is not proof it is running. A Windows process object outlives
+    # the process and is destroyed only once the last handle to it closes, so
+    # OpenProcess keeps succeeding on the pid of something that exited minutes
+    # ago while anyone still holds a handle -- and someone almost always does,
+    # namely whoever spawned it and has not reaped it. That is precisely the
+    # case this watchdog exists for: a parent killed by Force Quit or Task
+    # Manager runs no cleanup, so nothing is ever reaped and the worker would
+    # keep its GPU forever (#579).
+    #
+    # GetExitCodeProcess is the call that separates the two states. Its one
+    # documented ambiguity is a process that genuinely exits with code 259,
+    # which is indistinguishable from a running one -- and that error lands on
+    # the "alive" side, which is the side this function must fail towards.
+    try:
+        code = ctypes.c_ulong()
+        ok = kernel32.GetExitCodeProcess(handle, ctypes.byref(code))
+    finally:
         kernel32.CloseHandle(handle)
+    if not ok:
         return True
-    return ctypes.get_last_error() != ERROR_INVALID_PARAMETER
+    return code.value == STILL_ACTIVE
 
 
 _PARENT_POLL_SECONDS = 1.0

tests/test_logs_api.py

@@ -87,7 +87,12 @@ def test_zip_bundles_every_present_log(client, logs_dir):
 
 
 def test_zip_preserves_contents(client, logs_dir):
-    (logs_dir / "stemdeck.log").write_text("line one\nline two\n", encoding="utf-8")
+    # newline="" so the bytes on disk are the bytes written. Text mode
+    # translates \n to \r\n on Windows, and this test is about whether the
+    # endpoint preserves a file byte for byte -- a fixture that quietly rewrites
+    # itself cannot answer that, and made the assertion fail there for a reason
+    # having nothing to do with the endpoint (#580).
+    (logs_dir / "stemdeck.log").write_text("line one\nline two\n", encoding="utf-8", newline="")
     with zipfile.ZipFile(io.BytesIO(client.get("/api/logs.zip").content)) as z:
         assert z.read("stemdeck.log").decode() == "line one\nline two\n"
 

tests/test_pipeline_beatgrid.py

@@ -1,11 +1,13 @@
 from __future__ import annotations
 
 import json
+import shutil
 import wave
 
 import pytest
 from fastapi.testclient import TestClient
 
+from app.core.config import ffmpeg_executable
 from app.core.models import Job
 from app.core.registry import _jobs
 from app.pipeline.beatgrid import (
@@ -26,9 +28,33 @@ def _isolate_registry():
     _jobs.clear()
 
 
+# Resolved the way the app resolves it, so a portable install or a
+# STEMDECK_FFMPEG_DIR pointing at a bundled build counts as present. Computed
+# once: every test below would otherwise ask the filesystem the same question.
+_FFMPEG = shutil.which(ffmpeg_executable())
+
+
 @pytest.fixture
 def stems_dir(tmp_path, monkeypatch):
-    """A stems dir that `_load_audio_ffmpeg`'s JOBS_DIR containment check accepts."""
+    """A stems dir that `_load_audio_ffmpeg`'s JOBS_DIR containment check accepts.
+
+    Skips the whole test when ffmpeg is missing rather than letting it fail
+    downstream. `_decode_mono` is deliberately forgiving -- everything it feeds
+    is a display field, so it logs and returns None instead of raising -- which
+    means a missing binary and an undetectable beat grid arrive here as the same
+    `assert grid is not None`, and the real cause is only in a log record
+    nothing asserts on (#581).
+
+    The guard covers every test taking this fixture, not only the ones that go
+    red. The four `test_returns_none_*` cases passed without ffmpeg for the
+    wrong reason: they got their None from the decode failing, not from the
+    audio, so they could not have caught a regression in what they test.
+    """
+    if _FFMPEG is None:
+        pytest.skip(
+            "ffmpeg not found: install it, or point STEMDECK_FFMPEG_DIR at a "
+            "directory containing an ffmpeg binary"
+        )
     from app.pipeline import analyze as analyze_mod
 
     monkeypatch.setattr(analyze_mod, "JOBS_DIR", tmp_path)

tests/test_process_liveness.py

@@ -73,38 +73,83 @@ def test_posix_signal_zero_is_a_probe_not_a_kill(monkeypatch):
 # ── the Windows branch ───────────────────────────────────────────────
 
 
+STILL_ACTIVE = 259
+
+
 class _Kernel32:
     """Stands in for kernel32, so the Windows path runs anywhere."""
 
-    def __init__(self, handle: int, last_error: int = 0):
+    def __init__(
+        self,
+        handle: int,
+        last_error: int = 0,
+        exit_code: int = STILL_ACTIVE,
+        exit_code_ok: bool = True,
+    ):
         self._handle = handle
+        self._exit_code = exit_code
+        self._exit_code_ok = exit_code_ok
         self.last_error = last_error
         self.closed: list[int] = []
 
     def OpenProcess(self, _access, _inherit, _pid):  # noqa: N802 - Win32 name
         return self._handle
 
+    def GetExitCodeProcess(self, _handle, out):  # noqa: N802 - Win32 name
+        # `out` is what ctypes.byref produced; _obj is the c_ulong behind it.
+        out._obj.value = self._exit_code
+        return self._exit_code_ok
+
     def CloseHandle(self, handle):  # noqa: N802 - Win32 name
         self.closed.append(handle)
         return True
 
 
-def _run_windows_branch(monkeypatch, handle: int, last_error: int = 0):
+def _run_windows_branch(
+    monkeypatch,
+    handle: int,
+    last_error: int = 0,
+    exit_code: int = STILL_ACTIVE,
+    exit_code_ok: bool = True,
+):
     import ctypes
 
     monkeypatch.setattr(os, "name", "nt")
-    fake = _Kernel32(handle, last_error)
+    fake = _Kernel32(handle, last_error, exit_code, exit_code_ok)
     monkeypatch.setattr(ctypes, "WinDLL", lambda *_a, **_k: fake, raising=False)
     monkeypatch.setattr(ctypes, "get_last_error", lambda: fake.last_error, raising=False)
     return fake, process_exists(1234)
 
 
-def test_windows_an_open_handle_means_alive(monkeypatch):
+def test_windows_a_still_active_process_is_alive(monkeypatch):
     fake, alive = _run_windows_branch(monkeypatch, handle=42)
     assert alive is True
     assert fake.closed == [42], "the handle leaked; this runs on a watchdog timer"
 
 
+def test_windows_an_open_handle_is_not_enough_to_mean_alive(monkeypatch):
+    """The bug this file previously asserted as correct (#579).
+
+    A Windows process object outlives the process and dies only with the last
+    handle to it, so OpenProcess keeps succeeding on something that exited
+    while anyone still holds one -- and the un-reaped parent of a Force-Quit is
+    exactly that. Treating the open handle as proof of life meant the watchdog
+    never fired and the worker kept its GPU.
+    """
+    fake, alive = _run_windows_branch(monkeypatch, handle=42, exit_code=0)
+    assert alive is False
+    assert fake.closed == [42], "the handle leaked; this runs on a watchdog timer"
+
+
+def test_windows_an_unreadable_exit_code_means_alive(monkeypatch):
+    """Same ambiguity rule as everywhere else here: if the call that would
+    settle it fails, the answer is "alive". A watchdog must not shoot on a
+    question it could not ask."""
+    fake, alive = _run_windows_branch(monkeypatch, handle=42, exit_code=0, exit_code_ok=False)
+    assert alive is True
+    assert fake.closed == [42]
+
+
 def test_windows_invalid_parameter_means_dead(monkeypatch):
     """ERROR_INVALID_PARAMETER (87) is what OpenProcess returns for a pid that
     does not exist. It is the only failure that proves absence."""