diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index 1d79ba809..d51fb9a09 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -73,6 +73,30 @@ _HF_TOKEN_STATUS_SNIPPET = ( ) +def _windows_local_pid_record_line(pid_path: Path, ready_path: Path) -> str: + """Build the Git Bash prelude that records a Win32-stoppable PID. + + Python publishes the detached outer process's Win32 PID first, then touches + ``ready_path``. The inner Git Bash runner waits for that publication before + replacing the fallback with its own Win32 PID from /proc//winpid. + + Missing, malformed, or late mappings leave the valid outer PID untouched. + """ + pp = shlex.quote(pid_path.as_posix()) + rp = shlex.quote(ready_path.as_posix()) + return ( + "i=0; " + f"while [ ! -e {rp} ] && [ \"$i\" -lt 500 ]; do " + "i=$((i+1)); sleep 0.01; done; " + f"if [ -e {rp} ]; then " + "winpid=\"$(cat /proc/$$/winpid 2>/dev/null || true)\"; " + "case \"$winpid\" in ''|*[!0-9]*) ;; " + f"*) printf '%s\\n' \"$winpid\" > {pp} ;; esac; " + "fi; " + f"rm -f {rp}" + ) + + def _append_mlx_image_server_script(runner_lines: list[str]) -> None: """Write the MLX image API helper next to the tmux runner on remote hosts.""" script_path = Path(__file__).resolve().parents[1] / "scripts" / "mlx_image_server.py" @@ -978,15 +1002,18 @@ def setup_cookbook_routes() -> APIRouter: directly (simple commands only). Returns the launched job record.""" log_path = TMUX_LOG_DIR / f"{session_id}.log" pid_path = TMUX_LOG_DIR / f"{session_id}.pid" + pid_ready_path: Path | None = None bash = find_bash() if bash: # Run the existing bash wrapper verbatim through Git Bash, redirecting # all output to the log the poller reads. Paths handed to bash use # POSIX form + shell-quoting so drive paths / spaces survive. inner = TMUX_LOG_DIR / f"{session_id}_run.sh" - pp = shlex.quote(pid_path.as_posix()) + pid_ready_path = TMUX_LOG_DIR / f"{session_id}.pid.ready" + pid_ready_path.unlink(missing_ok=True) inner.write_text( - f"printf '%s\\n' \"$$\" > {pp}\n" + "\n".join(bash_lines) + "\n", + _windows_local_pid_record_line(pid_path, pid_ready_path) + "\n" + + "\n".join(bash_lines) + "\n", encoding="utf-8", ) lp = shlex.quote(log_path.as_posix()) @@ -1020,7 +1047,18 @@ def setup_cookbook_routes() -> APIRouter: env=env, **detached_popen_kwargs(), ) + # Publish a valid Win32 ancestor first. The Git Bash runner may then + # replace it with its own Win32 pid, but never before this fallback exists. pid_path.write_text(str(proc.pid), encoding="utf-8") + if pid_ready_path is not None: + try: + pid_ready_path.touch() + except OSError as e: + logger.warning( + "Could not publish Windows local PID handoff for %s: %s", + session_id, + e, + ) return {"pid": proc.pid, "log_path": str(log_path)} @router.post("/api/model/download") diff --git a/tests/test_cookbook_helpers.py b/tests/test_cookbook_helpers.py index bf6c47d4b..37620c88a 100644 --- a/tests/test_cookbook_helpers.py +++ b/tests/test_cookbook_helpers.py @@ -723,7 +723,12 @@ def test_local_windows_download_pid_tracks_inner_bash_and_stop_kills_tree(): routes_src = (Path(__file__).resolve().parents[1] / "routes" / "cookbook_routes.py").read_text(encoding="utf-8") running_src = (Path(__file__).resolve().parents[1] / "static" / "js" / "cookbookRunning.js").read_text(encoding="utf-8") - assert 'printf \'%s\\\\n\' \\"$$\\" > {pp}' in routes_src + # The Windows-local runner publishes Python's valid Win32 fallback before + # allowing Git Bash to replace it with /proc/$$/winpid. + assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in routes_src + assert "/proc/$$/winpid" in routes_src + assert "pid_ready_path.touch()" in routes_src + assert '\\"$$\\" > {pp}' not in routes_src assert "function Stop-Tree([int]$Id)" in running_src assert "('ParentProcessId = ' + $Id)" in running_src assert "Stop-Tree ([int]$p)" in running_src diff --git a/tests/test_cookbook_local_serve_pid_winpid.py b/tests/test_cookbook_local_serve_pid_winpid.py new file mode 100644 index 000000000..7038fccbf --- /dev/null +++ b/tests/test_cookbook_local_serve_pid_winpid.py @@ -0,0 +1,180 @@ +"""Behavioral regression coverage for Windows-local Cookbook PID recording.""" + +import os +import subprocess +import time +from pathlib import Path + +from routes.cookbook_routes import _windows_local_pid_record_line + + +ROOT = Path(__file__).resolve().parents[1] +COOKBOOK_ROUTES = ROOT / "routes" / "cookbook_routes.py" + + +def _fake_cat(tmp_path: Path, body: str) -> Path: + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + cat = fake_bin / "cat" + cat.write_text("#!/bin/sh\n" + body + "\n", encoding="utf-8") + cat.chmod(0o755) + return fake_bin + + +def _env_for(fake_bin: Path, **extra: str) -> dict[str, str]: + env = dict(os.environ) + env["PATH"] = str(fake_bin) + os.pathsep + env.get("PATH", "") + env.update(extra) + return env + + +def _run_pid_line( + pid_path: Path, + ready_path: Path, + fake_bin: Path, + **extra_env: str, +) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", "-c", _windows_local_pid_record_line(pid_path, ready_path)], + capture_output=True, + text=True, + env=_env_for(fake_bin, **extra_env), + timeout=10, + ) + + +def test_windows_local_pid_line_records_numeric_winpid_after_fallback(tmp_path): + pid_path = tmp_path / "serve.pid" + ready_path = tmp_path / "serve.pid.ready" + + pid_path.write_text("11111", encoding="utf-8") + ready_path.touch() + + cat_arg = tmp_path / "cat-arg.txt" + fake_bin = _fake_cat( + tmp_path, + 'printf "%s\\n" "$1" > "$FAKE_CAT_ARG"\n' + 'printf "%s\\n" "$FAKE_WINPID"', + ) + + result = _run_pid_line( + pid_path, + ready_path, + fake_bin, + FAKE_CAT_ARG=str(cat_arg), + FAKE_WINPID="42324", + ) + + assert result.returncode == 0, result.stderr + assert pid_path.read_text(encoding="utf-8").strip() == "42324" + assert not ready_path.exists() + + proc_path = cat_arg.read_text(encoding="utf-8").strip() + parts = proc_path.strip("/").split("/") + assert len(parts) == 3 + assert parts[0] == "proc" + assert parts[1].isdigit() + assert parts[2] == "winpid" + + +def test_windows_local_pid_line_waits_for_python_fallback_before_replacing(tmp_path): + pid_path = tmp_path / "serve.pid" + ready_path = tmp_path / "serve.pid.ready" + + fake_bin = _fake_cat( + tmp_path, + 'printf "%s\\n" "$FAKE_WINPID"', + ) + + proc = subprocess.Popen( + [ + "bash", + "-c", + _windows_local_pid_record_line(pid_path, ready_path), + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=_env_for(fake_bin, FAKE_WINPID="42324"), + ) + + # The inner shell has started, but Python has not published its fallback yet. + time.sleep(0.05) + assert proc.poll() is None + assert not pid_path.exists() + + # Simulate the post-Popen Python publication order. + pid_path.write_text("31100", encoding="utf-8") + ready_path.touch() + + stdout, stderr = proc.communicate(timeout=10) + + assert proc.returncode == 0, stderr or stdout + assert pid_path.read_text(encoding="utf-8").strip() == "42324" + assert not ready_path.exists() + + +def test_windows_local_pid_line_preserves_outer_pid_when_mapping_missing(tmp_path): + pid_path = tmp_path / "serve.pid" + ready_path = tmp_path / "serve.pid.ready" + + pid_path.write_text("31100", encoding="utf-8") + ready_path.touch() + + fake_bin = _fake_cat(tmp_path, "exit 1") + + result = _run_pid_line( + pid_path, + ready_path, + fake_bin, + ) + + assert result.returncode == 0, result.stderr + assert pid_path.read_text(encoding="utf-8").strip() == "31100" + assert not ready_path.exists() + + +def test_windows_local_pid_line_rejects_malformed_mapping(tmp_path): + pid_path = tmp_path / "serve.pid" + ready_path = tmp_path / "serve.pid.ready" + + pid_path.write_text("31100", encoding="utf-8") + ready_path.touch() + + fake_bin = _fake_cat( + tmp_path, + 'printf "not-a-win32-pid\\n"', + ) + + result = _run_pid_line( + pid_path, + ready_path, + fake_bin, + ) + + assert result.returncode == 0, result.stderr + assert pid_path.read_text(encoding="utf-8").strip() == "31100" + assert not ready_path.exists() + + +def test_local_windows_launcher_publishes_fallback_before_releasing_inner_runner(): + source = COOKBOOK_ROUTES.read_text(encoding="utf-8") + start = source.index(" def _launch_local_detached(") + end = source.index( + ' @router.post("/api/model/download")', + start, + ) + launcher = source[start:end] + + assert "_windows_local_pid_record_line(pid_path, pid_ready_path)" in launcher + assert "pid_ready_path.unlink(missing_ok=True)" in launcher + + fallback = launcher.index( + 'pid_path.write_text(str(proc.pid), encoding="utf-8")' + ) + release = launcher.index("pid_ready_path.touch()") + + assert fallback < release + + # Never write Git Bash's bare MSYS $$ to the session pid file. + assert '\\"$$\\" > {pp}' not in launcher