From 3d8c3646898653ada2cace161c9a55c21ff8282a Mon Sep 17 00:00:00 2001 From: hawktuahs <32615509+hawktuahs@users.noreply.github.com> Date: Thu, 4 Jun 2026 09:00:01 +0530 Subject: [PATCH] [Bash] Fix Windows cookbook background tasks (#676) * Fix Windows cookbook background tasks * Add Windows Cookbook reliability follow-ups --- core/platform_compat.py | 11 +++++ routes/cookbook_helpers.py | 33 ++++++++++++- routes/cookbook_routes.py | 30 ++++++------ static/js/cookbookRunning.js | 46 ++++++++++++------- ...okbook_dependency_completion_regression.py | 9 ++++ tests/test_cookbook_helpers.py | 30 ++++++++++++ tests/test_platform_compat.py | 26 ++++++++++- 7 files changed, 153 insertions(+), 32 deletions(-) diff --git a/core/platform_compat.py b/core/platform_compat.py index f971244..e2339ad 100644 --- a/core/platform_compat.py +++ b/core/platform_compat.py @@ -171,6 +171,15 @@ def _windows_bash_fallbacks() -> List[str]: return paths +def _is_windows_bash_stub(path: str) -> bool: + lowered = path.lower() + return ( + "system32\\bash.exe" in lowered + or "sysnative\\bash.exe" in lowered + or "windowsapps\\bash.exe" in lowered + ) + + def find_bash() -> Optional[str]: """Locate a real ``bash`` interpreter, or None. @@ -184,6 +193,8 @@ def find_bash() -> Optional[str]: return _BASH_CACHE _BASH_PROBED = True found = which_tool("bash") + if found and IS_WINDOWS and _is_windows_bash_stub(found): + found = None if not found and IS_WINDOWS: for cand in _windows_bash_fallbacks(): if os.path.exists(cand): diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index c60940a..9efb30d 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -2,6 +2,7 @@ Extracted from cookbook_routes.py; the routes module imports the symbols it needs.""" import logging +import ntpath import os import posixpath import re @@ -41,6 +42,15 @@ _GPU_LIST_RE = re.compile(r"^\d+(?:,\d+)*$") # only (no quotes, shell metacharacters, or spaces) since it lands in a shell # command. A leading ~ is expanded to $HOME at command-build time. _LOCAL_DIR_RE = re.compile(r"^~?/[A-Za-z0-9._/-]*$|^~$") +_WINDOWS_DRIVE_PATH_RE = re.compile(r"^[A-Za-z]:[\\/]") + + +def _git_bash_path(path: str) -> str: + m = re.match(r"^([A-Za-z]):[\\/](.*)$", path) + if not m: + return path + drive, rest = m.groups() + return f"/{drive.lower()}/{rest.replace(chr(92), '/')}" def _validate_repo_id(v: str | None) -> str: @@ -135,8 +145,11 @@ def _local_tooling_path_export(executable: str) -> str: # os.path.abspath("/opt/...") would incorrectly turn it into "D:\\opt\\...". if executable.startswith("/"): bin_dir = posixpath.dirname(executable) + elif _WINDOWS_DRIVE_PATH_RE.match(executable): + bin_dir = ntpath.dirname(executable) else: bin_dir = os.path.dirname(os.path.abspath(executable)) + bin_dir = _git_bash_path(bin_dir) # Escape for a double-quoted context: $PATH must still expand, but spaces # and shell metacharacters in the path must be preserved literally. esc = ( @@ -250,6 +263,17 @@ def _venv_safe_local_pip_install_cmd(cmd: str, *, local: bool, in_venv: bool) -> return shlex.join(stripped) +def _user_shell_path_bootstrap() -> list[str]: + return [ + 'ODYSSEUS_USER_SHELL="${SHELL:-}"', + 'if [ -n "$ODYSSEUS_USER_SHELL" ] && [ -x "$ODYSSEUS_USER_SHELL" ]; then', + ' ODYSSEUS_USER_PATH="$("$ODYSSEUS_USER_SHELL" -ic \'printf "__ODYSSEUS_PATH__%s\\n" "$PATH"\' 2>/dev/null | sed -n \'s/^__ODYSSEUS_PATH__//p\' | tail -n 1 || true)"', + ' if [ -n "$ODYSSEUS_USER_PATH" ]; then export PATH="$ODYSSEUS_USER_PATH:$PATH"; fi', + 'fi', + 'command -v python3 >/dev/null 2>&1 || python3() { python "$@"; }', + ] + + def _cached_model_scan_script(model_dirs: list[str] | None = None) -> str: """Build the standalone Python scanner used by /api/model/cached.""" lines = [ @@ -528,9 +552,16 @@ def _append_serve_preflight_exit_lines(runner_lines: list[str], *, keep_shell_op runner_lines.append('fi') -def _append_serve_exit_code_lines(runner_lines: list[str], *, keep_shell_open: bool) -> None: +def _append_serve_exit_code_lines( + runner_lines: list[str], + *, + keep_shell_open: bool, + is_pip_install: bool = False, +) -> None: """Append serve-runner lines that preserve and report the command exit code.""" runner_lines.append('ODYSSEUS_CMD_EXIT=$?') + if is_pip_install: + runner_lines.append('if [ $ODYSSEUS_CMD_EXIT -eq 0 ]; then echo ""; echo "DOWNLOAD_OK"; fi') if keep_shell_open: runner_lines.append('echo ""; echo "=== Process exited with code $ODYSSEUS_CMD_EXIT ==="; exec "${SHELL:-/bin/bash}"') else: diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index 7f2157b..56b95d6 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -38,7 +38,8 @@ from routes.cookbook_helpers import ( _ps_squote, _bash_squote, _validate_serve_cmd, _parse_serve_phase, _safe_env_prefix, _local_tooling_path_export, _append_serve_preflight_exit_lines, _append_serve_exit_code_lines, _append_llama_cpp_linux_accel_build_lines, _cached_model_scan_script, - _ollama_bind_from_cmd, _pip_install_fallback_chain, _pip_install_no_cache, _venv_safe_local_pip_install_cmd, + _ollama_bind_from_cmd, _pip_install_fallback_chain, _pip_install_no_cache, + _user_shell_path_bootstrap, _venv_safe_local_pip_install_cmd, ModelDownloadRequest, ServeRequest, ) @@ -294,15 +295,6 @@ def setup_cookbook_routes() -> APIRouter: safe_chmod(key_path.with_suffix(".pub"), 0o644) return {"ok": True, "public_key": _read_cookbook_public_key()} - def _user_shell_path_bootstrap() -> list[str]: - return [ - 'ODYSSEUS_USER_SHELL="${SHELL:-}"', - 'if [ -n "$ODYSSEUS_USER_SHELL" ] && [ -x "$ODYSSEUS_USER_SHELL" ]; then', - ' ODYSSEUS_USER_PATH="$("$ODYSSEUS_USER_SHELL" -ic \'printf "__ODYSSEUS_PATH__%s\\n" "$PATH"\' 2>/dev/null | sed -n \'s/^__ODYSSEUS_PATH__//p\' | tail -n 1 || true)"', - ' if [ -n "$ODYSSEUS_USER_PATH" ]; then export PATH="$ODYSSEUS_USER_PATH:$PATH"; fi', - 'fi', - ] - def _needs_binary(cmd: str, binary: str) -> bool: return bool(re.search(rf"(^|[\s;&|()]){re.escape(binary)}($|[\s;&|()])", cmd or "")) @@ -443,8 +435,8 @@ def setup_cookbook_routes() -> APIRouter: lines.append('export PATH="$HOME/.local/bin:$PATH"') # When Odysseus runs from a venv (e.g. native macOS install), put its bin # on PATH so the tmux shell finds the bundled `hf`/`python3` without an - # activated venv. Local bash runs only — meaningless over SSH/Windows. - if not req.remote_host and req.platform != "windows": + # activated venv. Local bash runs only — meaningless over SSH. + if not req.remote_host: lines.append(_local_tooling_path_export(sys.executable)) # Best-effort install hf CLI (always). hf_transfer (Rust parallel downloader) # is fast but flaky on large files — it tends to crash near the end at high @@ -982,6 +974,8 @@ def setup_cookbook_routes() -> APIRouter: ps_lines.append('Write-Host "ERROR: vLLM is not supported on Windows. Use Ollama or llama.cpp instead."') ps_lines.append('exit 1') ps_lines.append(req.cmd) + if is_pip_install: + ps_lines.append('if ($LASTEXITCODE -eq 0) { Write-Host ""; Write-Host "DOWNLOAD_OK" }') ps_lines.append('Write-Host ""') ps_lines.append('Write-Host "=== Process exited with code $LASTEXITCODE ==="') runner_path = TMUX_LOG_DIR / f"{session_id}_run.ps1" @@ -1167,10 +1161,18 @@ def setup_cookbook_routes() -> APIRouter: if local_windows: # Detached background process — no interactive shell to keep open. # Print the exit marker the status poller looks for, then stop. - _append_serve_exit_code_lines(runner_lines, keep_shell_open=False) + _append_serve_exit_code_lines( + runner_lines, + keep_shell_open=False, + is_pip_install=is_pip_install, + ) else: # Keep shell open after exit so user can see errors - _append_serve_exit_code_lines(runner_lines, keep_shell_open=True) + _append_serve_exit_code_lines( + runner_lines, + keep_shell_open=True, + is_pip_install=is_pip_install, + ) runner_path = TMUX_LOG_DIR / f"{session_id}_run.sh" runner_path.write_text("\n".join(runner_lines) + "\n", encoding="utf-8") diff --git a/static/js/cookbookRunning.js b/static/js/cookbookRunning.js index 971beff..36e93bb 100644 --- a/static/js/cookbookRunning.js +++ b/static/js/cookbookRunning.js @@ -728,37 +728,48 @@ export function _tmuxCmd(task, tmuxArgs) { } function _winSessionCmd(task, tmuxArgs) { - const sd = '$env:TEMP\\odysseus-sessions'; + const host = task.remoteHost; + const sd = host ? '$env:TEMP\\odysseus-sessions' : '$env:TEMP\\odysseus-tmux'; const sid = task.sessionId; const pf = _sshPrefix(_getPort(task)); - const host = task.remoteHost; if (tmuxArgs.includes('capture-pane')) { const lines = tmuxArgs.match(/-S\s*-?(\d+)/)?.[1] || '200'; - const ps = `Get-Content '${sd}\\${sid}.log' -Tail ${lines} -ErrorAction SilentlyContinue`; - return `ssh ${pf}${host} "powershell -Command \\"${ps}\\""`; + const ps = host + ? `Get-Content '${sd}\\${sid}.log' -Tail ${lines} -ErrorAction SilentlyContinue` + : `Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.log') -Tail ${lines} -ErrorAction SilentlyContinue`; + return host ? `ssh ${pf}${host} "powershell -Command \\"${ps}\\""` : `powershell -Command "${ps}"`; } if (tmuxArgs.includes('has-session')) { - const ps = `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Get-Process -Id $p -ErrorAction SilentlyContinue | Out-Null; if ($?) { exit 0 } else { exit 1 } } else { exit 1 }`; - return `ssh ${pf}${host} "powershell -Command \\"${ps}\\""`; + const ps = host + ? `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Get-Process -Id $p -ErrorAction SilentlyContinue | Out-Null; if ($?) { exit 0 } else { exit 1 } } else { exit 1 }` + : `$p = Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.pid') -ErrorAction SilentlyContinue; if ($p) { Get-Process -Id $p -ErrorAction SilentlyContinue | Out-Null; if ($?) { exit 0 } else { exit 1 } } else { exit 1 }`; + return host ? `ssh ${pf}${host} "powershell -Command \\"${ps}\\""` : `powershell -Command "${ps}"`; } if (tmuxArgs.includes('kill-session')) { - const ps = `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item '${sd}\\${sid}.*' -Force -ErrorAction SilentlyContinue`; - return `ssh ${pf}${host} "powershell -Command \\"${ps}\\""`; + const ps = host + ? `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item '${sd}\\${sid}.*' -Force -ErrorAction SilentlyContinue` + : `$p = Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.pid') -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.*') -Force -ErrorAction SilentlyContinue`; + return host ? `ssh ${pf}${host} "powershell -Command \\"${ps}\\""` : `powershell -Command "${ps}"`; } if (tmuxArgs.includes('send-keys') && tmuxArgs.includes('C-c')) { - const ps = `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -ErrorAction SilentlyContinue }`; - return `ssh ${pf}${host} "powershell -Command \\"${ps}\\""`; + const ps = host + ? `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -ErrorAction SilentlyContinue }` + : `$p = Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.pid') -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -ErrorAction SilentlyContinue }`; + return host ? `ssh ${pf}${host} "powershell -Command \\"${ps}\\""` : `powershell -Command "${ps}"`; } - return `ssh ${pf}${host} 'tmux ${tmuxArgs}' 2>/dev/null`; + return host ? `ssh ${pf}${host} 'tmux ${tmuxArgs}' 2>/dev/null` : `tmux ${tmuxArgs} 2>/dev/null`; } function _tmuxGracefulKill(task) { if (_isWindows(task)) { - const sd = '$env:TEMP\\odysseus-sessions'; + const host = task.remoteHost; + const sd = host ? '$env:TEMP\\odysseus-sessions' : '$env:TEMP\\odysseus-tmux'; const sid = task.sessionId; const pf = _sshPrefix(_getPort(task)); - const ps = `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item '${sd}\\${sid}.*' -Force -ErrorAction SilentlyContinue`; - return `ssh ${pf}${task.remoteHost} "powershell -Command \\"${ps}\\""`; + const ps = host + ? `$p = Get-Content '${sd}\\${sid}.pid' -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item '${sd}\\${sid}.*' -Force -ErrorAction SilentlyContinue` + : `$p = Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.pid') -ErrorAction SilentlyContinue; if ($p) { Stop-Process -Id $p -Force -ErrorAction SilentlyContinue }; Remove-Item (Join-Path $env:TEMP 'odysseus-tmux\\${sid}.*') -Force -ErrorAction SilentlyContinue`; + return host ? `ssh ${pf}${host} "powershell -Command \\"${ps}\\""` : `powershell -Command "${ps}"`; } if (task.remoteHost) { return `ssh ${_sshPrefix(_getPort(task))}${task.remoteHost} 'tmux send-keys -t ${task.sessionId} C-c 2>/dev/null; sleep 2; tmux kill-session -t ${task.sessionId} 2>/dev/null'`; @@ -2116,8 +2127,11 @@ export function _renderRunningTab() { }}); } if (_isWindows(task)) { - const sd = '$env:TEMP\\odysseus-sessions'; - const logCmd = `ssh ${_sshPrefix(_getPort(task))}${task.remoteHost} "powershell -Command \\"Get-Content '${sd}\\${task.sessionId}.log' -Wait\\""`; + const host = task.remoteHost; + const sd = host ? '$env:TEMP\\odysseus-sessions' : '$env:TEMP\\odysseus-tmux'; + const logCmd = host + ? `ssh ${_sshPrefix(_getPort(task))}${host} "powershell -Command \\"Get-Content '${sd}\\${task.sessionId}.log' -Wait\\""` + : `powershell -Command "Get-Content (Join-Path $env:TEMP 'odysseus-tmux\\${task.sessionId}.log') -Wait"`; items.push({ label: 'Copy log cmd', action: 'copy-tmux', custom: () => { _copyText(logCmd); }}); diff --git a/tests/test_cookbook_dependency_completion_regression.py b/tests/test_cookbook_dependency_completion_regression.py index 0ff90fd..b47e9b2 100644 --- a/tests/test_cookbook_dependency_completion_regression.py +++ b/tests/test_cookbook_dependency_completion_regression.py @@ -28,6 +28,15 @@ def test_background_status_poll_reconciles_into_local_tasks(): assert "completedDeps.forEach(t => _refreshDepsAfterInstall(t));" in source +def test_local_windows_session_commands_use_local_powershell_log_dir(): + source = _read("static/js/cookbookRunning.js") + + assert "const host = task.remoteHost;" in source + assert "host ? '$env:TEMP\\\\odysseus-sessions' : '$env:TEMP\\\\odysseus-tmux'" in source + assert "return host ? `ssh ${pf}${host}" in source + assert ": `powershell -Command \"${ps}\"`;" in source + + def test_dependency_install_payload_keeps_env_path_for_refresh(): source = _read("static/js/cookbook.js") diff --git a/tests/test_cookbook_helpers.py b/tests/test_cookbook_helpers.py index 6b8f425..de50dda 100644 --- a/tests/test_cookbook_helpers.py +++ b/tests/test_cookbook_helpers.py @@ -16,6 +16,7 @@ from routes.cookbook_helpers import ( _pip_install_fallback_chain, _ollama_bind_from_cmd, _safe_env_prefix, + _user_shell_path_bootstrap, _venv_safe_local_pip_install_cmd, _validate_gpus, _validate_repo_id, @@ -258,6 +259,17 @@ def test_pip_install_attempt_surfaces_stderr_on_failure(): assert "nonexistent" in combined.lower() or result.returncode != 0 +def test_local_tooling_path_export_converts_windows_paths_for_bash(): + line = _local_tooling_path_export(r"C:\Users\Jane Dev\.venv\Scripts\python.exe") + assert line == 'export PATH="/c/Users/Jane Dev/.venv/Scripts:$PATH"' + assert "C:" not in line + + +def test_user_shell_path_bootstrap_falls_back_to_python_on_windows_bash(): + script = "\n".join(_user_shell_path_bootstrap()) + assert 'command -v python3 >/dev/null 2>&1 || python3() { python "$@"; }' in script + + def test_serve_preflight_failure_keeps_tmux_pane_visible(): """Dependency preflight failures should remain visible in tmux output. @@ -290,6 +302,17 @@ def test_serve_runner_preserves_command_exit_code(): assert 'echo "=== Process exited with code $? ==="' not in script +def test_pip_serve_runner_emits_download_ok_before_exit_marker(): + """Dependency installs run through the serve wrapper need the download marker.""" + runner_lines = ["python3 -m pip install llama-cpp-python"] + _append_serve_exit_code_lines(runner_lines, keep_shell_open=False, is_pip_install=True) + script = "\n".join(runner_lines) + + assert 'echo "DOWNLOAD_OK"' in script + assert script.index('echo "DOWNLOAD_OK"') < script.index("=== Process exited with code") + assert 'exit "$ODYSSEUS_CMD_EXIT"' in script + + def test_validate_serve_cmd_accepts_vllm_kv_cache_dtype(): cmd = ( "CUDA_VISIBLE_DEVICES=0,1 vllm serve nvidia/Qwen3.6-35B-A3B-NVFP4 " @@ -410,6 +433,13 @@ def test_llama_cpp_linux_bootstrap_nvcc_without_cudart_warns_and_falls_back(): assert script.index(cpu_cmake) < script.index(no_toolchain_warn) +def test_llama_cpp_linux_bootstrap_uses_single_shell_continuations(): + runner_lines = [] + _append_llama_cpp_linux_accel_build_lines(runner_lines) + + assert not any(line.endswith("\\\\") for line in runner_lines) + + def test_llama_cpp_linux_bootstrap_keeps_cpu_fallback_when_no_gpu_toolchain(): runner_lines = [] _append_llama_cpp_linux_accel_build_lines(runner_lines) diff --git a/tests/test_platform_compat.py b/tests/test_platform_compat.py index 255974e..fbb43b8 100644 --- a/tests/test_platform_compat.py +++ b/tests/test_platform_compat.py @@ -1,6 +1,14 @@ """Regression tests for cross-platform helper behavior.""" -from core import platform_compat +import importlib.util +from pathlib import Path + + +_MODULE_PATH = Path(__file__).resolve().parents[1] / "core" / "platform_compat.py" +_SPEC = importlib.util.spec_from_file_location("platform_compat_under_test", _MODULE_PATH) +platform_compat = importlib.util.module_from_spec(_SPEC) +assert _SPEC and _SPEC.loader +_SPEC.loader.exec_module(platform_compat) def _reset_bash_cache(monkeypatch): @@ -35,3 +43,19 @@ def test_find_bash_checks_local_app_data_git_install(monkeypatch): monkeypatch.setattr(platform_compat.os.path, "exists", lambda path: path == expected) assert platform_compat.find_bash() == expected + + +def test_find_bash_skips_windows_wsl_stub(monkeypatch): + _reset_bash_cache(monkeypatch) + monkeypatch.setattr(platform_compat, "IS_WINDOWS", True) + + stub = r"C:\WINDOWS\system32\bash.exe" + expected = r"C:\Program Files\Git\bin\bash.exe" + monkeypatch.setattr( + platform_compat.shutil, + "which", + lambda name: stub if name == "bash" else None, + ) + monkeypatch.setattr(platform_compat.os.path, "exists", lambda path: path == expected) + + assert platform_compat.find_bash() == expected