diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index 429151f1a..24419c29c 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -1033,8 +1033,59 @@ def _append_llama_cpp_linux_accel_build_lines(runner_lines: list[str]) -> None: ODYSSEUS_LLAMA_SHIM_MARKER = "# Auto-generated by Odysseus Cookbook: a `llama-server` lookalike" +def _append_llama_server_resolution_definition_lines(runner_lines: list[str]) -> None: + """Define the helper shell function that discovers, reconciles and prioritizes llama-server.""" + marker = f"'{_bash_squote(ODYSSEUS_LLAMA_SHIM_MARKER)}'" + runner_lines.append('_odysseus_resolve_llama_server() {') + runner_lines.append(' _odysseus_llama_bin=""') + runner_lines.append(f' local _marker={marker}') + runner_lines.append(' local _saved_ifs="$IFS"') + runner_lines.append(" IFS=':'") + runner_lines.append(' local -a _parts') + runner_lines.append(' read -r -a _parts <<< "$PATH"') + runner_lines.append(' IFS="$_saved_ifs"') + runner_lines.append(' local _dir _cand') + runner_lines.append(' for _dir in "${_parts[@]}"; do') + runner_lines.append(' [ -n "$_dir" ] || continue') + runner_lines.append(' _cand="$_dir/llama-server"') + runner_lines.append(' [ -x "$_cand" ] || continue') + runner_lines.append(' if [ -f "$_cand" ] && [ ! -L "$_cand" ] && grep -Fxq "$_marker" "$_cand" 2>/dev/null; then') + runner_lines.append(' continue') + runner_lines.append(' fi') + runner_lines.append(' if "$_cand" --version >/dev/null 2>&1; then') + runner_lines.append(' _odysseus_llama_bin="$_cand"') + runner_lines.append(' break') + runner_lines.append(' fi') + runner_lines.append(' done') + runner_lines.append(' if [ -f "$HOME/bin/llama-server" ] && [ ! -L "$HOME/bin/llama-server" ] && grep -Fxq "$_marker" "$HOME/bin/llama-server" 2>/dev/null; then') + runner_lines.append(' if [ -n "$_odysseus_llama_bin" ]; then') + runner_lines.append(' echo "[odysseus] Native llama-server detected — retiring Odysseus compatibility shim in ~/bin"') + runner_lines.append(' rm -f "$HOME/bin/llama-server"') + runner_lines.append(' hash -r') + runner_lines.append(' elif python3 -c "import llama_cpp.server" 2>/dev/null; then') + runner_lines.append(' _odysseus_llama_bin="$HOME/bin/llama-server"') + runner_lines.append(' else') + runner_lines.append(' echo "[odysseus] Removing stale Odysseus llama-server shim (python3 -m llama_cpp.server unavailable)"') + runner_lines.append(' rm -f "$HOME/bin/llama-server"') + runner_lines.append(' hash -r') + runner_lines.append(' fi') + runner_lines.append(' fi') + runner_lines.append(' if [ -n "$_odysseus_llama_bin" ]; then') + runner_lines.append(' local _cand_dir') + runner_lines.append(' _cand_dir="$(dirname "$_odysseus_llama_bin")"') + runner_lines.append(' if [ "${PATH%%:*}" != "$_cand_dir" ]; then') + runner_lines.append(' export PATH="$_cand_dir:$PATH"') + runner_lines.append(' fi') + runner_lines.append(' hash -r') + runner_lines.append(' return 0') + runner_lines.append(' fi') + runner_lines.append(' return 1') + runner_lines.append('}') + + def _append_llama_server_shim_definition_lines(runner_lines: list[str]) -> None: """Define the helper shell function that generates the Odysseus compatibility shim.""" + _append_llama_server_resolution_definition_lines(runner_lines) marker = f"'{_bash_squote(ODYSSEUS_LLAMA_SHIM_MARKER)}'" runner_lines.append('_odysseus_create_llama_shim() {') runner_lines.append(' local _target="$HOME/bin/llama-server"') @@ -1104,36 +1155,14 @@ def _append_llama_server_shim_reconciliation_lines(runner_lines: list[str]) -> N - If python3 -m llama_cpp.server is unavailable, remove the stale shim and hash -r so the fallback/build path is triggered. - If no healthy native server exists and llama_cpp.server is working, preserve it. + - If a healthy native candidate exists on PATH (even later in PATH), prioritize it and + ensure the launched executable is the validated candidate. """ - marker = f"'{_bash_squote(ODYSSEUS_LLAMA_SHIM_MARKER)}'" runner_lines.append('[ -r ~/.config/odysseus-llama-cpp-env ] && . ~/.config/odysseus-llama-cpp-env') - runner_lines.append(f'if [ -f "$HOME/bin/llama-server" ] && [ ! -L "$HOME/bin/llama-server" ] && grep -Fxq {marker} "$HOME/bin/llama-server" 2>/dev/null; then') - runner_lines.append(' _odysseus_has_native=""') - runner_lines.append(' _odysseus_saved_ifs="$IFS"') - runner_lines.append(' IFS=\':\'') - runner_lines.append(' read -r -a _odysseus_path_parts <<< "$PATH"') - runner_lines.append(' IFS="$_odysseus_saved_ifs"') - runner_lines.append(' for _dir in "${_odysseus_path_parts[@]}"; do') - runner_lines.append(' [ -n "$_dir" ] || continue') - runner_lines.append(' case "$_dir" in') - runner_lines.append(' "$HOME/bin"|"$HOME/bin/") continue ;;') - runner_lines.append(' esac') - runner_lines.append(' _candidate="$_dir/llama-server"') - runner_lines.append(' if [ -x "$_candidate" ] && "$_candidate" --version >/dev/null 2>&1; then') - runner_lines.append(' _odysseus_has_native=1') - runner_lines.append(' break') - runner_lines.append(' fi') - runner_lines.append(' done') - runner_lines.append(' if [ -n "$_odysseus_has_native" ]; then') - runner_lines.append(' echo "[odysseus] Native llama-server detected — retiring Odysseus compatibility shim in ~/bin"') - runner_lines.append(' rm -f "$HOME/bin/llama-server"') - runner_lines.append(' hash -r') - runner_lines.append(' elif ! python3 -c "import llama_cpp.server" 2>/dev/null; then') - runner_lines.append(' echo "[odysseus] Removing stale Odysseus llama-server shim (python3 -m llama_cpp.server unavailable)"') - runner_lines.append(' rm -f "$HOME/bin/llama-server"') - runner_lines.append(' hash -r') - runner_lines.append(' fi') + runner_lines.append('if ! declare -F _odysseus_resolve_llama_server >/dev/null 2>&1; then') + _append_llama_server_resolution_definition_lines(runner_lines) runner_lines.append('fi') + runner_lines.append('_odysseus_resolve_llama_server || true') def _append_llama_server_finalization_lines(runner_lines: list[str], platform_hint: str = "") -> None: @@ -1148,10 +1177,14 @@ def _append_llama_server_finalization_lines(runner_lines: list[str], platform_hi if platform_hint else "ERROR: llama.cpp serving is not available after install/build attempts." ) - runner_lines.append('if ! command -v llama-server >/dev/null 2>&1 && python3 -c "import llama_cpp.server" 2>/dev/null; then') - runner_lines.append(' _odysseus_create_llama_shim') + runner_lines.append('if ! declare -F _odysseus_resolve_llama_server >/dev/null 2>&1; then') + _append_llama_server_resolution_definition_lines(runner_lines) runner_lines.append('fi') - runner_lines.append('if ! command -v llama-server &>/dev/null; then') + runner_lines.append('if [ -z "$_odysseus_llama_bin" ] && python3 -c "import llama_cpp.server" 2>/dev/null; then') + runner_lines.append(' _odysseus_create_llama_shim') + runner_lines.append(' _odysseus_resolve_llama_server || true') + runner_lines.append('fi') + runner_lines.append('if [ -z "$_odysseus_llama_bin" ]; then') runner_lines.append(f' echo "{err_msg}"') runner_lines.append(' ODYSSEUS_PREFLIGHT_EXIT=127') runner_lines.append('fi') diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index f915b5968..ae2667f65 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -2259,7 +2259,7 @@ def setup_cookbook_routes() -> APIRouter: runner_lines.append(' CMAKE_ARGS="-DGGML_BLAS=OFF -DGGML_LLAMAFILE=OFF" pip install \'llama-cpp-python[server]\' --no-build-isolation --no-cache-dir 2>&1 || true') runner_lines.append(' fi') _append_llama_server_finalization_lines(runner_lines, platform_hint="Termux") - runner_lines.append('elif ! command -v llama-server &>/dev/null; then') + runner_lines.append('elif [ -z "$_odysseus_llama_bin" ]; then') runner_lines.append(' echo "Native llama-server not found — building from source (one-time, may take a few minutes)..."') runner_lines.append(' mkdir -p ~/bin') runner_lines.append(' cd ~ && [ -d llama.cpp ] || git clone --depth 1 https://github.com/ggml-org/llama.cpp') @@ -2300,8 +2300,9 @@ def setup_cookbook_routes() -> APIRouter: runner_lines.append(' fi') runner_lines.append(' fi') runner_lines.append(' fi') + runner_lines.append(' _odysseus_resolve_llama_server || true') # If the native build failed, fall back to the Python bindings. - runner_lines.append(' if ! command -v llama-server &>/dev/null && ! python3 -c "import llama_cpp.server" 2>/dev/null; then') + runner_lines.append(' if [ -z "$_odysseus_llama_bin" ] && ! python3 -c "import llama_cpp.server" 2>/dev/null; then') runner_lines.append(' echo "llama-server build failed — installing Python bindings as fallback..."') runner_lines.append(f" {_pip_install_fallback_chain('llama-cpp-python[server]', python_cmd='pip')} || true") runner_lines.append(' fi') diff --git a/tests/test_cookbook_dependency_completion_regression.py b/tests/test_cookbook_dependency_completion_regression.py index 029257cff..2366e5098 100644 --- a/tests/test_cookbook_dependency_completion_regression.py +++ b/tests/test_cookbook_dependency_completion_regression.py @@ -1025,10 +1025,10 @@ def test_python_server_readiness_requires_llama_cpp_server_module(): # Termux fallback readiness check requires .server assert 'if ! python3 -c "import llama_cpp.server" 2>/dev/null; then' in routes_source # Build-failure fallback pip check requires .server - assert 'if ! command -v llama-server &>/dev/null && ! python3 -c "import llama_cpp.server" 2>/dev/null; then' in routes_source + assert 'if [ -z "$_odysseus_llama_bin" ] && ! python3 -c "import llama_cpp.server" 2>/dev/null; then' in routes_source # Shim reconciliation and post-install finalization require .server - assert 'if ! python3 -c "import llama_cpp.server" 2>/dev/null; then' in helpers_source - assert 'if ! command -v llama-server >/dev/null 2>&1 && python3 -c "import llama_cpp.server" 2>/dev/null; then' in helpers_source + assert 'elif python3 -c "import llama_cpp.server" 2>/dev/null; then' in helpers_source + assert 'if [ -z "$_odysseus_llama_bin" ] && python3 -c "import llama_cpp.server" 2>/dev/null; then' in helpers_source # GPU offload support checks still intentionally inspect base llama_cpp assert 'llama_cpp.llama_supports_gpu_offload()' in routes_source @@ -1092,8 +1092,17 @@ async def test_generated_runner_preflight_retires_stale_shim_before_backend_sele break assert endpoint is not None - http_req = Request({"type": "http", "method": "POST", "path": "/api/model/serve", "headers": []}) + from types import SimpleNamespace + http_req = Request({ + "type": "http", + "method": "POST", + "path": "/api/model/serve", + "headers": [], + "app": SimpleNamespace(state=SimpleNamespace(auth_manager=SimpleNamespace( + is_configured=True, is_admin=lambda user: user == "admin"))), + }) http_req.state.current_user = "admin" + http_req.state.local_model_authority = True req = ServeRequest( repo_id="test/repo", @@ -1112,7 +1121,7 @@ async def test_generated_runner_preflight_retires_stale_shim_before_backend_sele # Reconciliation must appear strictly BEFORE the Termux/backend check recon_idx = runner_script.find("Native llama-server detected — retiring Odysseus compatibility shim in ~/bin") termux_idx = runner_script.find("if [ -d /data/data/com.termux ]; then") - backend_idx = runner_script.find("elif ! command -v llama-server &>/dev/null; then") + backend_idx = runner_script.find('elif [ -z "$_odysseus_llama_bin" ]; then') assert recon_idx != -1, "Reconciliation block missing from generated runner" assert termux_idx != -1, "Termux check missing from generated runner" assert backend_idx != -1, "Backend selection missing from generated runner" @@ -1124,3 +1133,222 @@ async def test_generated_runner_preflight_retires_stale_shim_before_backend_sele assert not stale_shim.exists(), "Stale ~/bin/llama-server must have been retired by the generated preflight" assert "RUN_NATIVE: -m /dummy/model.gguf --port 8085" in res.stdout assert "Native llama-server not found — building from source" not in res.stdout + + +def test_broken_first_native_candidate_healthy_second_candidate_executes_healthy(tmp_path): + """Regression finding P2: When PATH contains an earlier broken candidate followed by a + healthy candidate, reconciliation and launch must select and execute the healthy candidate.""" + from core.platform_compat import git_bash_path + broken_dir = tmp_path / "broken_bin" + _write_mock_native(broken_dir / "llama-server", succeeds=False) + + healthy_dir = tmp_path / "healthy_bin" + _write_mock_native(healthy_dir / "llama-server", succeeds=True) + + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + + lines = [ + f'export PATH="$HOME/bin:{git_bash_path(broken_dir)}:{git_bash_path(healthy_dir)}:/usr/bin:/bin"', + ] + _append_llama_server_shim_reconciliation_lines(lines) + lines.append('echo "RESOLVED=$_odysseus_llama_bin"') + lines.append('command -v llama-server') + lines.append('llama-server -m /dummy/model.gguf --port 8080') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert not shim_file.exists(), "Stale Odysseus shim must be retired" + assert f"RESOLVED={git_bash_path(healthy_dir / 'llama-server')}" in res.stdout + assert str(healthy_dir / "llama-server") in res.stdout + assert "RUN_NATIVE: -m /dummy/model.gguf --port 8080" in res.stdout + assert "error: missing libllama.so" not in res.stdout + + +def test_paths_with_spaces_broken_first_healthy_second_launch(tmp_path): + """Regression: Paths containing spaces must be safely resolved, prioritized and launched.""" + from core.platform_compat import git_bash_path + broken_dir = tmp_path / "broken path with spaces/bin" + _write_mock_native(broken_dir / "llama-server", succeeds=False) + + healthy_dir = tmp_path / "healthy path with spaces/bin" + _write_mock_native(healthy_dir / "llama-server", succeeds=True) + + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + + model_dir = tmp_path / "models with spaces" + model_dir.mkdir(parents=True, exist_ok=True) + model_file = model_dir / "my model.gguf" + model_file.write_text("dummy", encoding="utf-8") + + lines = [ + f'export PATH="$HOME/bin:{git_bash_path(broken_dir)}:{git_bash_path(healthy_dir)}:/usr/bin:/bin"', + ] + _append_llama_server_shim_reconciliation_lines(lines) + lines.append(f'llama-server -m "{git_bash_path(model_file)}" --port 8080') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert not shim_file.exists(), "Stale shim must be retired" + assert f"RUN_NATIVE: -m {git_bash_path(model_file)} --port 8080" in res.stdout + assert "error: missing libllama.so" not in res.stdout + + +def test_command_hash_invalidation_switches_from_cached_broken_to_healthy(tmp_path): + """Regression: Command cache invalidation (`hash -r`) must prevent bash from reusing a + previously cached lookup to an earlier broken executable.""" + from core.platform_compat import git_bash_path + broken_dir = tmp_path / "broken_bin" + _write_mock_native(broken_dir / "llama-server", succeeds=False) + + healthy_dir = tmp_path / "healthy_bin" + _write_mock_native(healthy_dir / "llama-server", succeeds=True) + + lines = [ + # Populate bash hash cache with broken binary + f'export PATH="{git_bash_path(broken_dir)}:/usr/bin:/bin"', + 'llama-server >/dev/null 2>&1 || true', + 'hash', + # Now add healthy candidate later in PATH and run reconciliation + f'export PATH="$HOME/bin:{git_bash_path(broken_dir)}:{git_bash_path(healthy_dir)}:/usr/bin:/bin"', + ] + _append_llama_server_shim_reconciliation_lines(lines) + lines.append('llama-server -m /dummy/test.gguf') + lines.append('hash') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert "RUN_NATIVE: -m /dummy/test.gguf" in res.stdout + # Verify hash table ended with healthy binary, not broken + assert str(healthy_dir / "llama-server") in res.stdout + + +def test_broken_native_candidate_falls_back_to_healthy_python_compatibility_shim(tmp_path): + """Regression: A broken native candidate must not shadow or retire a healthy Python shim.""" + from core.platform_compat import git_bash_path + broken_dir = tmp_path / "broken_bin" + _write_mock_native(broken_dir / "llama-server", succeeds=False) + + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + + mock_py_dir = tmp_path / "mock_bin" + _write_mock_python3(mock_py_dir / "python3", succeeds=True) + + lines = [ + f'export PATH="$HOME/bin:{git_bash_path(broken_dir)}:{git_bash_path(mock_py_dir)}:/usr/bin:/bin"', + ] + _append_llama_server_shim_reconciliation_lines(lines) + lines.append('echo "RESOLVED=$_odysseus_llama_bin"') + lines.append('command -v llama-server') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert shim_file.exists(), "Healthy Python compatibility shim must be preserved" + assert f"RESOLVED={git_bash_path(shim_file)}" in res.stdout + assert str(shim_file) in res.stdout + + +def test_broken_native_and_missing_python_backend_sets_preflight_127(tmp_path): + """Regression: When the only native candidates are broken and Python server is unavailable, + finalization must fail closed with ODYSSEUS_PREFLIGHT_EXIT=127 rather than claiming readiness.""" + from core.platform_compat import git_bash_path + broken_dir = tmp_path / "broken_bin" + _write_mock_native(broken_dir / "llama-server", succeeds=False) + + mock_py_dir = tmp_path / "mock_bin" + _write_mock_python3(mock_py_dir / "python3", succeeds=False) + + lines = [ + 'ODYSSEUS_PREFLIGHT_EXIT=""', + f'export PATH="$HOME/bin:{git_bash_path(broken_dir)}:{git_bash_path(mock_py_dir)}:/usr/bin:/bin"', + ] + _append_llama_server_shim_definition_lines(lines) + _append_llama_server_shim_reconciliation_lines(lines) + _append_llama_server_finalization_lines(lines) + lines.append('echo "EXIT=$ODYSSEUS_PREFLIGHT_EXIT"') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert "EXIT=127" in res.stdout + + +@pytest.mark.asyncio +async def test_generated_runner_broken_first_healthy_second_e2e(monkeypatch, tmp_path): + """Integration regression: The full generated Cookbook runner script must launch the healthy + native candidate even when an earlier broken candidate exists on PATH.""" + from unittest.mock import AsyncMock, MagicMock + from types import SimpleNamespace + from routes.cookbook_helpers import ServeRequest + import routes.cookbook_routes as cookbook_routes + from starlette.requests import Request + from core.platform_compat import git_bash_path + + fake_home = tmp_path / "fake_home" + stale_shim = fake_home / "bin/llama-server" + _write_odysseus_shim(stale_shim) + + broken_bin = fake_home / "broken/bin/llama-server" + _write_mock_native(broken_bin, succeeds=False) + + healthy_bin = fake_home / "healthy/bin/llama-server" + _write_mock_native(healthy_bin, succeeds=True) + + tmux_log_dir = tmp_path / "tmux_logs" + tmux_log_dir.mkdir(parents=True) + monkeypatch.setattr(cookbook_routes, "TMUX_LOG_DIR", tmux_log_dir) + monkeypatch.setattr(cookbook_routes, "require_admin", lambda request: None) + monkeypatch.setattr(cookbook_routes, "_binary_available", AsyncMock(return_value=True)) + monkeypatch.setattr(cookbook_routes, "load_stored_hf_token", lambda **kwargs: "") + monkeypatch.setattr("core.database.SessionLocal", MagicMock()) + + class _Proc: + returncode = 0 + stderr = None + async def wait(self): + return 0 + monkeypatch.setattr(cookbook_routes.asyncio, "create_subprocess_shell", AsyncMock(return_value=_Proc())) + + router = cookbook_routes.setup_cookbook_routes() + endpoint = None + for route in router.routes: + if route.path == "/api/model/serve" and "POST" in route.methods: + endpoint = route.endpoint + break + assert endpoint is not None + + http_req = Request({ + "type": "http", + "method": "POST", + "path": "/api/model/serve", + "headers": [], + "app": SimpleNamespace(state=SimpleNamespace(auth_manager=SimpleNamespace( + is_configured=True, is_admin=lambda user: user == "admin"))), + }) + http_req.state.current_user = "admin" + http_req.state.local_model_authority = True + + req = ServeRequest( + repo_id="test/repo", + cmd="llama-server -m /dummy/model.gguf --port 8090", + remote_host=None, + ) + + resp = await endpoint(http_req, req) + assert resp["ok"] is True + session_id = resp["session_id"] + runner_file = tmux_log_dir / f"{session_id}_run.sh" + assert runner_file.exists() + + # Broken is earlier on PATH than healthy + res = _run_shell( + fake_home, + [str(runner_file)], + path=f"{git_bash_path(fake_home / 'bin')}:{git_bash_path(broken_bin.parent)}:{git_bash_path(healthy_bin.parent)}:/usr/bin:/bin", + ) + assert res.returncode == 0 + assert not stale_shim.exists(), "Stale shim must have been retired" + assert "RUN_NATIVE: -m /dummy/model.gguf --port 8090" in res.stdout + assert "error: missing libllama.so" not in res.stdout + assert "Native llama-server not found — building from source" not in res.stdout