From a7c03f85bf22fe0f827e4f5258921ed499dc165f Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira Date: Mon, 17 Aug 2026 00:59:13 +0100 Subject: [PATCH 01/13] fix(cookbook): recover from stale llama-server shims Reconcile Odysseus-owned Python compatibility shims when llama_cpp.server is no longer available, allowing healthy native llama-server binaries in PATH to be discovered and executed without shadowing. Fixes #6079 --- routes/cookbook_helpers.py | 127 +++++ routes/cookbook_routes.py | 66 +-- ...okbook_dependency_completion_regression.py | 509 ++++++++++++++++++ 3 files changed, 648 insertions(+), 54 deletions(-) diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index e724b2dc1..a854cc7b7 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -1030,6 +1030,133 @@ def _append_llama_cpp_linux_accel_build_lines(runner_lines: list[str]) -> None: runner_lines.append(' fi # end _odysseus_have_prebuilt guard') +ODYSSEUS_LLAMA_SHIM_MARKER = "# Auto-generated by Odysseus Cookbook: a `llama-server` lookalike" + + +def _append_llama_server_shim_definition_lines(runner_lines: list[str]) -> None: + """Define the helper shell function that generates the Odysseus compatibility shim.""" + marker = f"'{_bash_squote(ODYSSEUS_LLAMA_SHIM_MARKER)}'" + runner_lines.append('_odysseus_create_llama_shim() {') + runner_lines.append(' local _target="$HOME/bin/llama-server"') + runner_lines.append(f' local _marker={marker}') + runner_lines.append(' if [ -e "$_target" ] || [ -L "$_target" ]; then') + runner_lines.append(' if [ -f "$_target" ] && [ ! -L "$_target" ] && grep -Fxq "$_marker" "$_target" 2>/dev/null; then') + runner_lines.append(' : # existing Odysseus shim — safe to replace') + runner_lines.append(' else') + runner_lines.append(' echo "[odysseus] WARNING: $_target already exists and is not an Odysseus-managed shim; leaving untouched." >&2') + runner_lines.append(' return 1') + runner_lines.append(' fi') + runner_lines.append(' fi') + runner_lines.append(' mkdir -p "$HOME/bin" || return 1') + runner_lines.append(' local _tmp') + runner_lines.append(' _tmp="$(mktemp "$HOME/bin/.llama-server.odysseus.tmp.XXXXXX")" || {') + runner_lines.append(' echo "[odysseus] ERROR: unable to create secure temporary llama-server shim in $HOME/bin" >&2') + runner_lines.append(' return 1') + runner_lines.append(' }') + runner_lines.append(" if ! cat > \"$_tmp\" <<'_ODY_LLAMA_SHIM_EOF'") + runner_lines.append('#!/usr/bin/env bash') + runner_lines.append(ODYSSEUS_LLAMA_SHIM_MARKER) + runner_lines.append('# that translates the native CLI to `python -m llama_cpp.server`.') + runner_lines.append('# Lets cookbook-generated launch commands run unchanged on hosts') + runner_lines.append('# where only the pip llama-cpp-python package is installed.') + runner_lines.append('ARGS=()') + runner_lines.append('while [ $# -gt 0 ]; do') + runner_lines.append(' case "$1" in') + runner_lines.append(' -ngl|--gpu-layers|--n-gpu-layers) ARGS+=(--n_gpu_layers "$2"); shift 2 ;;') + runner_lines.append(' -c|--ctx-size) ARGS+=(--n_ctx "$2"); shift 2 ;;') + runner_lines.append(' -b|--batch-size) ARGS+=(--n_batch "$2"); shift 2 ;;') + runner_lines.append(' -ub|--ubatch-size) shift 2 ;; # llama-cpp-python has no separate ubatch') + runner_lines.append(' --flash-attn) ARGS+=(--flash_attn true); shift 2 ;;') + runner_lines.append(' --cache-type-k) ARGS+=(--type_k "$2"); shift 2 ;;') + runner_lines.append(' --cache-type-v) ARGS+=(--type_v "$2"); shift 2 ;;') + runner_lines.append(' --n-cpu-moe) ARGS+=(--n_cpu_moe "$2"); shift 2 ;;') + runner_lines.append(' --mmproj) ARGS+=(--clip_model_path "$2"); shift 2 ;;') + runner_lines.append(' --image-max-tokens) shift 2 ;; # native-only') + runner_lines.append(' --no-mmap) ARGS+=(--no_mmap true); shift ;;') + runner_lines.append(' --no-warmup) shift ;; # native-only') + runner_lines.append(' --chat-template) ARGS+=(--chat_format "$2"); shift 2 ;;') + runner_lines.append(' --fit|--split-mode|--tensor-split|--main-gpu|--parallel) shift 2 ;; # native-only') + runner_lines.append(' --mlock) ARGS+=(--use_mlock true); shift ;;') + runner_lines.append(' *) ARGS+=("$1"); shift ;;') + runner_lines.append(' esac') + runner_lines.append('done') + runner_lines.append('exec python3 -m llama_cpp.server "${ARGS[@]}"') + runner_lines.append('_ODY_LLAMA_SHIM_EOF') + runner_lines.append(' then') + runner_lines.append(' rm -f -- "$_tmp"') + runner_lines.append(' return 1') + runner_lines.append(' fi') + runner_lines.append(' chmod +x "$_tmp" || { rm -f -- "$_tmp"; return 1; }') + runner_lines.append(' mv -f -- "$_tmp" "$_target" || { rm -f -- "$_tmp"; return 1; }') + runner_lines.append(' hash -r') + runner_lines.append(' echo "[odysseus] Created llama-server shim → python -m llama_cpp.server (no native binary needed)"') + runner_lines.append('}') + + +def _append_llama_server_shim_reconciliation_lines(runner_lines: list[str]) -> None: + """Reconcile an existing ~/bin/llama-server if it is owned by Odysseus Cookbook. + + Invariants: + - Never touch an arbitrary user-owned executable or symlink. + - If ~/bin/llama-server is an Odysseus generated shim: + - If a native llama-server is present and healthy elsewhere on PATH, remove the shim + so the native binary is used. + - 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. + """ + 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('fi') + + +def _append_llama_server_finalization_lines(runner_lines: list[str], platform_hint: str = "") -> None: + """Reconcile post-install launcher availability and enforce fail-closed invariant. + + Guarantees that if python3 -m llama_cpp.server is usable but the `llama-server` + command is unresolvable, the compatibility launcher is created. If no usable + launcher exists after all build/install attempts, sets ODYSSEUS_PREFLIGHT_EXIT=127. + """ + err_msg = ( + f"ERROR: llama.cpp serving is not available on {platform_hint} after install attempts." + 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('fi') + runner_lines.append('if ! command -v llama-server &>/dev/null; then') + runner_lines.append(f' echo "{err_msg}"') + runner_lines.append(' ODYSSEUS_PREFLIGHT_EXIT=127') + runner_lines.append('fi') + + def _llama_cpp_rebuild_cmd(update_source: bool = False) -> str: """Shell command that clears the Cookbook-managed llama.cpp build. diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index d51fb9a09..a041029ff 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -57,9 +57,11 @@ from routes.cookbook_helpers import ( _pip_install_no_cache, _user_shell_path_bootstrap, _venv_safe_local_pip_install_cmd, _diagnose_serve_output, run_ssh_command_async, _ollama_bind_from_cmd, _pip_install_fallback_chain, _pip_install_no_cache, - _user_shell_path_bootstrap, _venv_safe_local_pip_install_cmd, _append_pip_install_runner_lines, _pip_install_command_without_break_system_packages, _normalize_llama_cpp_python_cache_types, + _append_llama_server_shim_definition_lines, + _append_llama_server_shim_reconciliation_lines, + _append_llama_server_finalization_lines, ModelDownloadRequest, ServeRequest, ) @@ -2102,7 +2104,7 @@ def setup_cookbook_routes() -> APIRouter: ps_lines.append('}') elif "llama_cpp" in req.cmd or "llama-server" in req.cmd: ps_lines.append('# Auto-install llama-cpp-python if missing') - ps_lines.append('try { python -c "import llama_cpp" 2>$null } catch {}') + ps_lines.append('try { python -c "import llama_cpp.server" 2>$null } catch {}') ps_lines.append('if ($LASTEXITCODE -ne 0) {') ps_lines.append(' Write-Host "Installing llama-cpp-python..."') ps_lines.append(' python -m pip install llama-cpp-python[server]') @@ -2188,13 +2190,16 @@ def setup_cookbook_routes() -> APIRouter: # ollama is found (otherwise macOS falls back to a slow source build). # /opt/homebrew = Apple Silicon, /usr/local = Intel; harmless on Linux. runner_lines.append('export PATH="$HOME/.local/bin:$HOME/bin:$HOME/llama.cpp/build/bin:/opt/homebrew/bin:/usr/local/bin:$PATH"') + _append_llama_server_shim_definition_lines(runner_lines) + _append_llama_server_shim_reconciliation_lines(runner_lines) runner_lines.append('if [ -d /data/data/com.termux ]; then') runner_lines.append(' # Termux: no native build — use the Python bindings (CPU).') - runner_lines.append(' if ! python3 -c "import llama_cpp" 2>/dev/null; then') + runner_lines.append(' if ! python3 -c "import llama_cpp.server" 2>/dev/null; then') runner_lines.append(' pkg install -y cmake 2>/dev/null') runner_lines.append(' pip install numpy diskcache jinja2 2>/dev/null') 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(' echo "Native llama-server not found — building from source (one-time, may take a few minutes)..."') runner_lines.append(' mkdir -p ~/bin') @@ -2212,8 +2217,7 @@ def setup_cookbook_routes() -> APIRouter: # would reuse the bad settings and fail again. CMAKE_BUILD_TYPE is # explicit so the binary is optimized (Metal auto-enables on macOS). runner_lines.append(' cd ~/llama.cpp && rm -rf build && cmake -B build -DCMAKE_BUILD_TYPE=Release \\') - runner_lines.append(' && cmake --build build -j"$NPROC" --target llama-server \\') - runner_lines.append(' && ln -sf ~/llama.cpp/build/bin/llama-server ~/bin/llama-server') + runner_lines.append(' && cmake --build build -j"$NPROC" --target llama-server') runner_lines.append(' else') _append_llama_cpp_linux_accel_build_lines(runner_lines) runner_lines.append(' fi') @@ -2237,58 +2241,12 @@ def setup_cookbook_routes() -> APIRouter: runner_lines.append(' fi') runner_lines.append(' fi') runner_lines.append(' fi') - # SHORT-CIRCUIT before the build/pip fallback: if the - # native binary is missing but llama_cpp Python is already - # installed, drop a wrapper at ~/bin/llama-server that - # translates llama-server CLI args to llama_cpp.server's - # underscore-style flags. The user's serve command stays - # `llama-server ...` and "just works" — no build, no cmake, - # no second install. This is the path that unblocks every - # remote where pip-installed llama-cpp-python is already - # working but Cookbook used to insist on a native binary. - runner_lines.append(' if ! command -v llama-server >/dev/null 2>&1 && python3 -c "import llama_cpp" 2>/dev/null; then') - runner_lines.append(' mkdir -p ~/bin') - runner_lines.append(' cat > ~/bin/llama-server <<\'_ODY_LLAMA_SHIM_EOF\'') - runner_lines.append('#!/usr/bin/env bash') - runner_lines.append('# Auto-generated by Odysseus Cookbook: a `llama-server` lookalike') - runner_lines.append('# that translates the native CLI to `python -m llama_cpp.server`.') - runner_lines.append('# Lets cookbook-generated launch commands run unchanged on hosts') - runner_lines.append('# where only the pip llama-cpp-python package is installed.') - runner_lines.append('ARGS=()') - runner_lines.append('while [ $# -gt 0 ]; do') - runner_lines.append(' case "$1" in') - runner_lines.append(' -ngl|--gpu-layers|--n-gpu-layers) ARGS+=(--n_gpu_layers "$2"); shift 2 ;;') - runner_lines.append(' -c|--ctx-size) ARGS+=(--n_ctx "$2"); shift 2 ;;') - runner_lines.append(' -b|--batch-size) ARGS+=(--n_batch "$2"); shift 2 ;;') - runner_lines.append(' -ub|--ubatch-size) shift 2 ;; # llama-cpp-python has no separate ubatch') - runner_lines.append(' --flash-attn) ARGS+=(--flash_attn true); shift 2 ;;') - runner_lines.append(' --cache-type-k) ARGS+=(--type_k "$2"); shift 2 ;;') - runner_lines.append(' --cache-type-v) ARGS+=(--type_v "$2"); shift 2 ;;') - runner_lines.append(' --n-cpu-moe) ARGS+=(--n_cpu_moe "$2"); shift 2 ;;') - runner_lines.append(' --mmproj) ARGS+=(--clip_model_path "$2"); shift 2 ;;') - runner_lines.append(' --image-max-tokens) shift 2 ;; # native-only') - runner_lines.append(' --no-mmap) ARGS+=(--no_mmap true); shift ;;') - runner_lines.append(' --no-warmup) shift ;; # native-only') - runner_lines.append(' --chat-template) ARGS+=(--chat_format "$2"); shift 2 ;;') - runner_lines.append(' --fit|--split-mode|--tensor-split|--main-gpu|--parallel) shift 2 ;; # native-only') - runner_lines.append(' --mlock) ARGS+=(--use_mlock true); shift ;;') - runner_lines.append(' *) ARGS+=("$1"); shift ;;') - runner_lines.append(' esac') - runner_lines.append('done') - runner_lines.append('exec python3 -m llama_cpp.server "${ARGS[@]}"') - runner_lines.append('_ODY_LLAMA_SHIM_EOF') - runner_lines.append(' chmod +x ~/bin/llama-server') - runner_lines.append(' echo "[odysseus] Created llama-server shim → python -m llama_cpp.server (no native binary needed)"') - runner_lines.append(' fi') - runner_lines.append(' # 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" 2>/dev/null; then') + # 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(' 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') - runner_lines.append(' if ! command -v llama-server &>/dev/null && ! python3 -c "import llama_cpp" 2>/dev/null; then') - runner_lines.append(' echo "ERROR: llama.cpp serving is not available after install/build attempts."') - runner_lines.append(' ODYSSEUS_PREFLIGHT_EXIT=127') - runner_lines.append(' fi') + _append_llama_server_finalization_lines(runner_lines) runner_lines.append('fi') elif re.search(r"\bollama\s+serve\b", req.cmd): handled_ollama_serve = True diff --git a/tests/test_cookbook_dependency_completion_regression.py b/tests/test_cookbook_dependency_completion_regression.py index bf1efc389..6dd1248d2 100644 --- a/tests/test_cookbook_dependency_completion_regression.py +++ b/tests/test_cookbook_dependency_completion_regression.py @@ -1,4 +1,14 @@ +import os +import subprocess from pathlib import Path +import pytest +from core.platform_compat import find_bash, git_bash_path +from routes.cookbook_helpers import ( + ODYSSEUS_LLAMA_SHIM_MARKER, + _append_llama_server_shim_definition_lines, + _append_llama_server_shim_reconciliation_lines, + _append_llama_server_finalization_lines, +) ROOT = Path(__file__).resolve().parents[1] @@ -115,3 +125,502 @@ def test_local_dependency_probe_refreshes_user_site_visibility(): # deps (realesrgan) probe as not-installed until a restart. See #4810. assert "if user_site and os.path.isdir(user_site):" in source assert "site.addsitedir(user_site)" in source + + +def test_llama_runner_path_preserves_standard_user_bin_order(): + """The runner PATH must preserve standard user $HOME/bin precedence while + relying on ownership-aware reconciliation to retire Odysseus shims when + native builds exist.""" + source = _read("routes/cookbook_routes.py") + + assert ( + 'export PATH="$HOME/.local/bin:$HOME/bin:$HOME/llama.cpp/build/bin:/opt/homebrew/bin:/usr/local/bin:$PATH"' + in source + ) + + +def _run_shell(tmp_path, lines, cwd=None, path=None, extra_env=None): + from core.platform_compat import find_bash, git_bash_path + bash = find_bash() or "bash" + env = dict(os.environ, HOME=git_bash_path(tmp_path)) + if path is not None: + env["PATH"] = path + if extra_env: + env.update(extra_env) + return subprocess.run( + [bash, "-c", "\n".join(lines)], + cwd=str(cwd) if cwd else None, + env=env, + capture_output=True, + text=True, + ) + + +def _write_odysseus_shim(path: Path) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + f"#!/usr/bin/env bash\n{ODYSSEUS_LLAMA_SHIM_MARKER}\nexec python3 -m llama_cpp.server \"$@\"\n", + encoding="utf-8", + ) + path.chmod(0o755) + + +def _write_mock_native(path: Path, succeeds: bool = True) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + if succeeds: + content = '#!/usr/bin/env bash\nif [ "$1" = "--version" ]; then echo "version: 0.1.0-dev"; exit 0; fi\necho "RUN_NATIVE: $@"\nexit 0\n' + else: + content = '#!/usr/bin/env bash\necho "error: missing libllama.so" >&2\nexit 127\n' + path.write_text(content, encoding="utf-8") + path.chmod(0o755) + + +def _write_mock_python3(path: Path, succeeds: bool = True) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"#!/usr/bin/env bash\nexit {0 if succeeds else 1}\n", encoding="utf-8") + path.chmod(0o755) + + +def test_odysseus_create_llama_shim_never_overwrites_user_regular_file(tmp_path): + """Creation boundary: never overwrite a non-executable user-owned regular file.""" + user_file = tmp_path / "bin/llama-server" + user_file.parent.mkdir(parents=True, exist_ok=True) + original_content = "user configuration or binary data without marker" + user_file.write_text(original_content, encoding="utf-8") + + lines = [] + _append_llama_server_shim_definition_lines(lines) + lines.append('_odysseus_create_llama_shim') + + res = _run_shell(tmp_path, lines) + assert res.returncode != 0, "Creation should return non-zero when target is a user file" + assert user_file.read_text(encoding="utf-8") == original_content, "User file must remain byte-for-byte unchanged" + + +def test_odysseus_create_llama_shim_never_overwrites_user_symlink(tmp_path): + """Creation boundary: never follow or overwrite a user-owned symlink.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir(parents=True, exist_ok=True) + target_file = tmp_path / "user_target.txt" + target_file.write_text("precious user target content", encoding="utf-8") + user_symlink = bin_dir / "llama-server" + user_symlink.symlink_to(target_file) + + lines = [] + _append_llama_server_shim_definition_lines(lines) + lines.append('_odysseus_create_llama_shim') + + res = _run_shell(tmp_path, lines) + assert res.returncode != 0, "Creation should return non-zero when target is a user symlink" + assert user_symlink.is_symlink(), "Symlink must remain intact" + assert target_file.read_text(encoding="utf-8") == "precious user target content", "Target file must not be modified" + + +def test_odysseus_create_llama_shim_never_overwrites_dangling_symlink(tmp_path): + """Creation boundary: never overwrite a dangling user-owned symlink.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir(parents=True, exist_ok=True) + dangling_symlink = bin_dir / "llama-server" + dangling_symlink.symlink_to(tmp_path / "nonexistent_target") + + lines = [] + _append_llama_server_shim_definition_lines(lines) + lines.append('_odysseus_create_llama_shim') + + res = _run_shell(tmp_path, lines) + assert res.returncode != 0, "Creation should return non-zero when target is a dangling symlink" + assert dangling_symlink.is_symlink(), "Dangling symlink must remain intact" + + +def test_odysseus_create_llama_shim_safely_replaces_existing_odysseus_shim(tmp_path): + """Creation boundary: safely replace an existing Odysseus-owned compatibility shim.""" + shim_file = tmp_path / "bin/llama-server" + shim_file.parent.mkdir(parents=True, exist_ok=True) + shim_file.write_text(f"#!/bin/bash\n{ODYSSEUS_LLAMA_SHIM_MARKER}\n# old shim body\n", encoding="utf-8") + + lines = [] + _append_llama_server_shim_definition_lines(lines) + lines.append('_odysseus_create_llama_shim') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + new_content = shim_file.read_text(encoding="utf-8") + assert ODYSSEUS_LLAMA_SHIM_MARKER in new_content + assert "exec python3 -m llama_cpp.server" in new_content + + +def test_odysseus_create_llama_shim_secure_temp_prevents_symlink_attack(tmp_path): + """Creation boundary: secure mktemp prevents predictable $$ temp symlink attacks. + + Given: + - A victim file at ~/victim containing DO_NOT_TOUCH + - A pre-created symlink at ~/bin/.llama-server.odysseus.tmp.$$ pointing to ~/victim + - _odysseus_create_llama_shim executed within that exact same bash PID ($$) + + Prove: + - ~/victim remains unmodified + - Pre-existing crafted symlink is NOT followed or overwritten + - Legitimate shim is successfully created at ~/bin/llama-server via secure temp path + """ + lines = [ + 'victim="$HOME/victim"', + 'printf "DO_NOT_TOUCH\\n" > "$victim"', + 'mkdir -p "$HOME/bin"', + 'old_tmp="$HOME/bin/.llama-server.odysseus.tmp.$$";', + 'ln -s "$victim" "$old_tmp"', + ] + _append_llama_server_shim_definition_lines(lines) + lines.append('_odysseus_create_llama_shim') + lines.append('res=$?') + lines.append('echo "CREATION_RES=$res"') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert "CREATION_RES=0" in res.stdout + + victim_file = tmp_path / "victim" + assert victim_file.exists() + assert victim_file.read_text(encoding="utf-8") == "DO_NOT_TOUCH\n", "Victim file must remain untouched" + + target_shim = tmp_path / "bin/llama-server" + assert target_shim.exists() + assert ODYSSEUS_LLAMA_SHIM_MARKER in target_shim.read_text(encoding="utf-8") + assert target_shim.stat().st_mode & 0o111 != 0, "Shim must be executable" + + +def test_stale_odysseus_shim_removed_when_python_server_unusable(tmp_path): + """Scenario 1: Stale Odysseus shim + missing Python server -> owned shim removed.""" + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + + lines = [] + _append_llama_server_shim_reconciliation_lines(lines) + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert not shim_file.exists(), "Stale Odysseus shim must be removed when python server is unavailable" + + +def test_healthy_odysseus_shim_retained_when_no_native_server(tmp_path): + """Scenario 2: Healthy Odysseus shim + no native server -> shim preserved.""" + 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 = [] + _append_llama_server_shim_reconciliation_lines(lines) + + res = _run_shell(tmp_path, lines, path=f"{git_bash_path(mock_py_dir)}:{os.environ.get('PATH', '')}") + assert res.returncode == 0 + assert shim_file.exists(), "Healthy Odysseus shim must be preserved when no native server is present" + + +def test_reconciliation_does_not_retire_healthy_shim_for_broken_native_binary(tmp_path): + """Reconciliation health check: a broken native candidate executable must NOT retire + a healthy Odysseus Python shim.""" + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + _write_mock_native(tmp_path / "llama.cpp/build/bin/llama-server", succeeds=False) + mock_py_dir = tmp_path / "mock_bin" + _write_mock_python3(mock_py_dir / "python3", succeeds=True) + + lines = [f'export PATH="$HOME/llama.cpp/build/bin:{git_bash_path(mock_py_dir)}:$PATH"'] + _append_llama_server_shim_reconciliation_lines(lines) + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert shim_file.exists(), "Healthy Odysseus shim must NOT be retired when native binary is broken" + + +def test_odysseus_shim_retired_when_native_server_healthy(tmp_path): + """Scenario 3: Odysseus shim + healthy native server available -> native server wins.""" + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + _write_mock_native(tmp_path / "llama.cpp/build/bin/llama-server", succeeds=True) + + lines = ['export PATH="$HOME/bin:$HOME/llama.cpp/build/bin:$PATH"'] + _append_llama_server_shim_reconciliation_lines(lines) + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert not shim_file.exists(), "Odysseus shim must be retired when healthy native server is available" + + +def test_user_owned_bin_llama_server_never_touched(tmp_path): + """Scenario 4: Arbitrary user ~/bin/llama-server -> untouched and not deprioritized.""" + user_bin = tmp_path / "bin/llama-server" + user_bin.parent.mkdir(parents=True, exist_ok=True) + user_bin.write_text("#!/bin/bash\n# Custom user build script\nexec /opt/custom/llama-server\n", encoding="utf-8") + user_bin.chmod(0o755) + + _write_mock_native(tmp_path / "llama.cpp/build/bin/llama-server", succeeds=True) + + lines = ['export PATH="$HOME/bin:$HOME/llama.cpp/build/bin:$PATH"'] + _append_llama_server_shim_reconciliation_lines(lines) + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + assert user_bin.exists(), "User-owned ~/bin/llama-server must never be deleted" + assert "Custom user build script" in user_bin.read_text(encoding="utf-8") + + +def test_reconciliation_ignores_native_binary_outside_effective_path(tmp_path): + """Native candidate outside effective PATH must NOT be considered.""" + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + # Native binary placed in /usr_custom/bin which is NOT in PATH + _write_mock_native(tmp_path / "usr_custom/bin/llama-server", succeeds=True) + mock_py_dir = tmp_path / "mock_bin" + _write_mock_python3(mock_py_dir / "python3", succeeds=True) + + lines = [] + _append_llama_server_shim_reconciliation_lines(lines) + + # PATH only contains ~/bin and mock_py_dir — usr_custom is omitted + path = f"{git_bash_path(tmp_path / 'bin')}:{git_bash_path(mock_py_dir)}" + res = _run_shell(tmp_path, lines, path=path) + assert res.returncode == 0 + assert shim_file.exists(), "Shim must NOT be retired by native binary outside effective PATH" + + +def test_reconciliation_skips_home_bin_with_or_without_trailing_slash(tmp_path): + """Reconciliation must exclude $HOME/bin and $HOME/bin/ from native candidate discovery.""" + 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 = [] + _append_llama_server_shim_reconciliation_lines(lines) + + # PATH has ~/bin/ with trailing slash + path = f"{git_bash_path(tmp_path / 'bin')}/:{git_bash_path(mock_py_dir)}" + res = _run_shell(tmp_path, lines, path=path) + assert res.returncode == 0 + assert shim_file.exists(), "Shim must NOT rediscover itself when PATH has trailing slash in $HOME/bin/" + + +def test_reconciliation_does_not_discover_cwd_executable_when_path_omits_dot(tmp_path): + """Reconciliation must NOT synthesize an empty/trailing PATH element that searches cwd. + + Given: + - Current working directory contains an executable `llama-server` + - Healthy Odysseus Python shim in ~/bin/llama-server + - PATH does NOT contain '.' or cwd + - No other healthy native binary on PATH + + Prove: + - Reconciliation does not discover ./llama-server + - The healthy Odysseus shim in ~/bin/llama-server is preserved + """ + 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) + + cwd_dir = tmp_path / "workdir" + _write_mock_native(cwd_dir / "llama-server", succeeds=True) + + lines = [] + _append_llama_server_shim_reconciliation_lines(lines) + + path = f"{git_bash_path(tmp_path / 'bin')}:{git_bash_path(mock_py_dir)}:/usr/bin:/bin" + res = _run_shell(tmp_path, lines, cwd=cwd_dir, path=path) + assert res.returncode == 0 + assert shim_file.exists(), "Healthy Odysseus shim must NOT be retired by finding ./llama-server in cwd" + + +def test_reconciliation_discovers_healthy_native_on_explicit_other_path_entry(tmp_path): + """Reconciliation must discover healthy native binaries placed on explicit PATH entries.""" + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + custom_opt_bin = tmp_path / "custom_opt/bin" + _write_mock_native(custom_opt_bin / "llama-server", succeeds=True) + + lines = [] + _append_llama_server_shim_reconciliation_lines(lines) + + path = f"{git_bash_path(tmp_path / 'bin')}:{git_bash_path(custom_opt_bin)}:/usr/bin:/bin" + res = _run_shell(tmp_path, lines, path=path) + assert res.returncode == 0 + assert not shim_file.exists(), "Odysseus shim must be retired when healthy native server is on explicit PATH entry" + + +def test_reconciliation_safely_iterates_path_with_glob_metacharacters(tmp_path): + """Reconciliation must not perform pathname expansion / globbing on PATH components. + + Given: + - A directory path containing shell glob characters: ~/custom[dir]/bin + - A native binary ~/custom[dir]/bin/llama-server + - A working directory containing matching glob entries + - An existing Odysseus shim in ~/bin/llama-server + + Prove: + - Reconciliation correctly parses the literal path without expanding [dir] + - Discovers the healthy native server and retires the Odysseus shim + """ + shim_file = tmp_path / "bin/llama-server" + _write_odysseus_shim(shim_file) + + glob_dir = tmp_path / "custom[dir]/bin" + _write_mock_native(glob_dir / "llama-server", succeeds=True) + + cwd_dir = tmp_path / "cwd" + cwd_dir.mkdir(parents=True, exist_ok=True) + (cwd_dir / "customd").mkdir(parents=True, exist_ok=True) + (cwd_dir / "customi").mkdir(parents=True, exist_ok=True) + + lines = [] + _append_llama_server_shim_reconciliation_lines(lines) + + path = f"{git_bash_path(tmp_path / 'bin')}:{git_bash_path(glob_dir)}:/usr/bin:/bin" + res = _run_shell(tmp_path, lines, cwd=cwd_dir, path=path) + assert res.returncode == 0 + assert not shim_file.exists(), "Reconciliation must discover literal bracketed PATH without globbing" + + +def test_fallback_install_success_creates_usable_launcher(tmp_path): + """Scenario 5: No native + no Python server -> fallback install succeeds + -> usable compatibility launcher exists and resolves command -v llama-server.""" + 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(mock_py_dir)}:$PATH"'] + _append_llama_server_shim_definition_lines(lines) + _append_llama_server_finalization_lines(lines) + lines.append('command -v llama-server') + + res = _run_shell(tmp_path, lines) + assert res.returncode == 0 + created_shim = tmp_path / "bin/llama-server" + assert created_shim.exists() + assert ODYSSEUS_LLAMA_SHIM_MARKER in created_shim.read_text(encoding="utf-8") + assert str(tmp_path / "bin/llama-server") in res.stdout + + +def test_failed_fallback_sets_preflight_exit_127(tmp_path): + """Scenario 6: No native + failed fallback installation -> ODYSSEUS_PREFLIGHT_EXIT=127.""" + lines = [ + 'ODYSSEUS_PREFLIGHT_EXIT=""', + 'export PATH="$HOME/bin:$PATH"', + ] + _append_llama_server_shim_definition_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 + + +def test_python_server_readiness_requires_llama_cpp_server_module(): + """Scenario 7 & 8: Python server readiness requires `import llama_cpp.server`, + while GPU offload capability checks still use base `import llama_cpp`.""" + routes_source = _read("routes/cookbook_routes.py") + helpers_source = _read("routes/cookbook_helpers.py") + + # Windows fallback readiness check requires .server + assert 'try { python -c "import llama_cpp.server" 2>$null } catch {}' in routes_source + # 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 + # 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 + + # GPU offload support checks still intentionally inspect base llama_cpp + assert 'llama_cpp.llama_supports_gpu_offload()' in routes_source + + +def test_darwin_native_build_does_not_force_write_bin_symlink(): + """Darwin source build must not overwrite ~/bin/llama-server with a forced symlink.""" + source = _read("routes/cookbook_routes.py") + + assert "&& ln -sf ~/llama.cpp/build/bin/llama-server ~/bin/llama-server" not in source + assert 'cmake --build build -j"$NPROC" --target llama-server' in source + + +@pytest.mark.asyncio +async def test_generated_runner_preflight_retires_stale_shim_before_backend_selection(monkeypatch, tmp_path): + """Integration regression: The actual generated Cookbook runner must execute + ownership reconciliation BEFORE evaluating the backend selection (`if Termux ... elif ! command -v llama-server`). + + Given: + - Executable stale Odysseus shim in ~/bin/llama-server (with canonical marker) + - Healthy native llama-server in ~/llama.cpp/build/bin/llama-server + - ~/bin before ~/llama.cpp/build/bin on PATH + + Prove: + 1. Generated runner contains reconciliation shell lines strictly before the backend branching + 2. Executing the generated runner script retires the stale ~/bin/llama-server shim + 3. Resolves to the healthy native binary without attempting source build or pip fallback. + """ + from unittest.mock import AsyncMock, MagicMock + from routes.cookbook_helpers import ServeRequest + import routes.cookbook_routes as cookbook_routes + from starlette.requests import Request + + fake_home = tmp_path / "fake_home" + stale_shim = fake_home / "bin/llama-server" + _write_odysseus_shim(stale_shim) + native_bin = fake_home / "llama.cpp/build/bin/llama-server" + _write_mock_native(native_bin, succeeds=True) + + # Mock environment to generate the real runner file with isolated DB + 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": []}) + http_req.state.current_user = "admin" + + req = ServeRequest( + repo_id="test/repo", + cmd="llama-server -m /dummy/model.gguf --port 8085", + 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() + runner_script = runner_file.read_text(encoding="utf-8") + + # Assert structural ordering in the generated script: + # 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") + 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" + assert recon_idx < termux_idx < backend_idx, "Reconciliation must execute strictly before backend selection" + + # Execute the generated runner preflight in a real subshell + res = _run_shell(fake_home, [str(runner_file)], path=f"{git_bash_path(fake_home / 'bin')}:{git_bash_path(fake_home / 'llama.cpp/build/bin')}:/usr/bin:/bin") + assert res.returncode == 0 + 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 From 50b05bf54f84920e96eaff4943b784e2b6aa2d33 Mon Sep 17 00:00:00 2001 From: harshit-ojha0324 <92013456+harshit-ojha0324@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:48:28 -0400 Subject: [PATCH 02/13] fix(mcp): close the HTTP transport when a remote connect fails _connect_http entered streamablehttp_client and ClientSession on an AsyncExitStack but never closed it when initialize or list_tools failed. The half-open transport was left to the garbage collector, which exited its AnyIO cancel scope from the wrong task, and the event loop kept spinning: about 18% of a core per failed connect, growing with each one, until restart. Close the stack in the connecting task on any failure or cancellation, the same way _connect_sse already does. Fixes #5518 --- src/mcp_manager.py | 20 ++++++++--- tests/test_mcp_manager.py | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/mcp_manager.py b/src/mcp_manager.py index 53112009d..b47d0e1d7 100644 --- a/src/mcp_manager.py +++ b/src/mcp_manager.py @@ -10,6 +10,7 @@ import logging import os import re import asyncio +import contextlib from typing import Any, Dict, List, Optional, Set, Tuple from src.database import McpServer, SessionLocal @@ -383,12 +384,21 @@ class McpManager: provider = build_provider(server_id, url, on_redirect=_on_redirect) stack = AsyncExitStack() - transport = await stack.enter_async_context(streamablehttp_client(url, auth=provider)) - read_stream, write_stream, _get_session_id = transport - session = await stack.enter_async_context(ClientSession(read_stream, write_stream)) - await session.initialize() + try: + transport = await stack.enter_async_context(streamablehttp_client(url, auth=provider)) + read_stream, write_stream, _get_session_id = transport + session = await stack.enter_async_context(ClientSession(read_stream, write_stream)) + await session.initialize() - tools_result = await session.list_tools() + tools_result = await session.list_tools() + except BaseException: + # Close the half-open transport here, in the task that entered + # it. Left for the garbage collector, its AnyIO cancel scope is + # exited from the wrong task and the event loop keeps spinning + # (#5518). Its own teardown error would only mask the real one. + with contextlib.suppress(Exception): + await stack.aclose() + raise tools = [] for tool in tools_result.tools: tools.append({ diff --git a/tests/test_mcp_manager.py b/tests/test_mcp_manager.py index a879f95eb..11e655422 100644 --- a/tests/test_mcp_manager.py +++ b/tests/test_mcp_manager.py @@ -39,3 +39,74 @@ def test_http_transport_routes_to_start_http_connect(): result = asyncio.run(mgr.connect_server("id1", "n", "http", url="https://x/mcp")) assert result == "ROUTED" m.assert_called_once() + + +def _patch_http_transport(monkeypatch, initialize): + """Fake the Streamable HTTP transport and record, for each close, whether + it ran in the task that opened it (AnyIO requires that).""" + import contextlib + import mcp + import mcp.client.streamable_http as streamable_http + import src.mcp_oauth as mcp_oauth + + closes = [] + + @contextlib.asynccontextmanager + async def fake_client(url, auth=None): + opener = asyncio.current_task() + try: + yield (object(), object(), lambda: None) + finally: + closes.append(asyncio.current_task() is opener) + + class FakeSession: + def __init__(self, *a): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + FakeSession.initialize = initialize + monkeypatch.setattr(streamable_http, "streamablehttp_client", fake_client) + monkeypatch.setattr(mcp, "ClientSession", FakeSession) + monkeypatch.setattr(mcp_oauth, "build_provider", lambda *a, **k: None) + return closes + + +def test_failed_http_connect_closes_transport_in_its_own_task(monkeypatch): + """#5518: a failed connect left its transport for the GC, which exited the + AnyIO cancel scope from the wrong task and kept the event loop spinning.""" + async def initialize(self): + raise RuntimeError("Session terminated") + + closes = _patch_http_transport(monkeypatch, initialize) + mgr = McpManager() + + ok = asyncio.run(mgr._start_http_connect("s1", "remote", "http://x/mcp/", wait=2)) + + assert ok is False + assert mgr._connections["s1"]["status"] == "error" + assert "Session terminated" in mgr._connections["s1"]["error"] + assert closes == [True] + assert "s1" not in mgr._stacks + + +def test_cancelled_http_connect_closes_transport(monkeypatch): + """Deleting a server mid-connect cancels the connect task; the transport + must still be closed rather than leaked.""" + async def initialize(self): + await asyncio.sleep(3600) + + closes = _patch_http_transport(monkeypatch, initialize) + mgr = McpManager() + + async def run(): + await mgr._start_http_connect("s1", "remote", "http://x/mcp/", wait=0.05) + await mgr.disconnect_server("s1") + await asyncio.sleep(0) + + asyncio.run(run()) + assert closes == [True] From ad3e309074e0d75fa73d57aa9a178b8f98603b67 Mon Sep 17 00:00:00 2001 From: robyroro <41092915+robyroro@users.noreply.github.com> Date: Thu, 8 Oct 2026 06:48:41 +0300 Subject: [PATCH 03/13] fix(chats): preserve saved conversations during tidy --- routes/session_routes.py | 47 +----- src/session_actions.py | 59 +------- tests/test_chat_tidy_content_preservation.py | 148 +++++++++++++++++++ tests/test_session_actions_cleanup.py | 7 +- tests/test_session_list_owner_scope.py | 22 +-- 5 files changed, 180 insertions(+), 103 deletions(-) create mode 100644 tests/test_chat_tidy_content_preservation.py diff --git a/routes/session_routes.py b/routes/session_routes.py index fa687ee14..c47be42e2 100644 --- a/routes/session_routes.py +++ b/routes/session_routes.py @@ -1305,7 +1305,7 @@ def setup_session_routes( def auto_sort_sessions(request: Request, skip_llm: bool = False): """Use AI to categorize all sessions into folders. - Phase 1 deletes empty/throwaway sessions and Phase 2 asks the LLM + Phase 1 deletes empty/Incognito sessions and Phase 2 asks the LLM to assign folders. When `skip_llm=true` the endpoint returns after Phase 1 — used by the "Tidy (no AI)" UI affordance so users can clean junk without spending tokens. @@ -1315,21 +1315,11 @@ def setup_session_routes( single_user_mode = not user and _auth_disabled() user_sessions = session_manager.get_sessions_for_user(user) - # Delete empty and throwaway sessions before sorting + # Saved conversations are never disposable based on their title or prompt. from core.database import ChatMessage as DbMsg db = SessionLocal() deleted_empty = 0 deleted_throwaway = 0 - # Names that indicate a throwaway/test session (case-insensitive exact or prefix match) - _THROWAWAY_NAMES = { - "test", "testing", "asdf", "asd", "hello", "hi", "hey", - "yo", "sup", "hola", "hii", "hiii", "heyo", - "foo", "bar", "baz", "tmp", "temp", "scratch", "untitled", - "new chat", "delete", "remove", "junk", "trash", "xxx", - "abc", "qwerty", "blah", "stuff", "whatever", "idk", - "ok", "lol", "bruh", "hmm", "hm", "meh", - } - _THROWAWAY_MAX_MESSAGES = 4 # only delete if <= this many messages try: rows_q = db.query(DbSession).filter(DbSession.archived == False) if user: @@ -1338,15 +1328,11 @@ def setup_session_routes( rows_q = rows_q.filter(DbSession.owner == user) rows = rows_q.limit(2000).all() folder_map = {r.id: r.folder for r in rows} - # Precompute per-session message counts in TWO aggregate queries + # Precompute per-session message counts in one aggregate query # instead of 1–3 queries PER session — with many chats the per-row # loop was doing thousands of round-trips and blowing the timeout. from sqlalchemy import func as _sa_func _counts = dict(db.query(DbMsg.session_id, _sa_func.count(DbMsg.id)).group_by(DbMsg.session_id).all()) - _asst_counts = dict( - db.query(DbMsg.session_id, _sa_func.count(DbMsg.id)) - .filter(DbMsg.role == "assistant").group_by(DbMsg.session_id).all() - ) cleanup_now = utcnow_naive() for row in rows: # Never delete important sessions @@ -1356,9 +1342,10 @@ def setup_session_routes( if (row.name or "").strip() == "Incognito": should_delete = True deleted_throwaway += 1 - db.delete(row) if hasattr(session_manager, 'delete_session'): session_manager.delete_session(row.id) + else: + db.delete(row) continue if is_session_recently_active(row, now=cleanup_now): continue @@ -1367,31 +1354,11 @@ def setup_session_routes( if msg_count == 0: should_delete = True deleted_empty += 1 - elif msg_count <= _THROWAWAY_MAX_MESSAGES: - name = (row.name or "").strip().lower() - # Check first user message content (AI renames sessions, so - # "hi" becomes "Casual Greeting Exchange" — name alone won't match) - first_msg = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id, DbMsg.role == "user" - ).order_by(DbMsg.timestamp).first() - first_text = (first_msg[0] or "").strip().lower() if first_msg else "" - # Count assistant messages — if user sent something but AI never replied, it's dead - assistant_count = _asst_counts.get(row.id, 0) - if name in _THROWAWAY_NAMES or name.startswith("chat:") or first_text in _THROWAWAY_NAMES: - should_delete = True - deleted_throwaway += 1 - # Single user message with no AI response = dead session - elif msg_count == 1 and assistant_count == 0: - should_delete = True - deleted_throwaway += 1 - # Short phrase (1-3 words) with no real AI conversation (<=2 msgs) - elif msg_count <= 2 and first_text and len(first_text.split()) <= 3 and len(first_text) <= 40: - should_delete = True - deleted_throwaway += 1 if should_delete: - db.delete(row) if hasattr(session_manager, 'delete_session'): session_manager.delete_session(row.id) + else: + db.delete(row) if deleted_empty or deleted_throwaway: db.commit() logger.info(f"Auto-sort: deleted {deleted_empty} empty + {deleted_throwaway} throwaway sessions") diff --git a/src/session_actions.py b/src/session_actions.py index 072bb4c06..d59979cec 100644 --- a/src/session_actions.py +++ b/src/session_actions.py @@ -12,16 +12,6 @@ from datetime import datetime, timedelta, timezone logger = logging.getLogger(__name__) -# Names that indicate a throwaway/test session -_THROWAWAY_NAMES = { - "test", "testing", "asdf", "asd", "hello", "hi", "hey", - "yo", "sup", "hola", "hii", "hiii", "heyo", - "foo", "bar", "baz", "tmp", "temp", "scratch", "untitled", - "new chat", "delete", "remove", "junk", "trash", "xxx", - "abc", "qwerty", "blah", "stuff", "whatever", "idk", - "ok", "lol", "bruh", "hmm", "hm", "meh", -} -_THROWAWAY_MAX_MESSAGES = 4 _FRESH_EMPTY_SESSION_GRACE = timedelta(minutes=10) _FRESH_SESSION_GRACE = _FRESH_EMPTY_SESSION_GRACE @@ -54,14 +44,15 @@ def is_session_recently_active(row, now=None, grace=_FRESH_SESSION_GRACE) -> boo async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bool = True) -> str: - """Run session cleanup + (optional) AI folder sort for the given owner. + """Remove empty/Incognito sessions and optionally organize saved chats. Args: owner: user whose sessions to process - skip_llm: when True, do only Phase 1 (delete empty/throwaway sessions); + skip_llm: when True, do only Phase 1 (delete empty/Incognito sessions); skip Phase 2 (AI folder assignment). Used by the built-in daily background sweep so it never burns LLM tokens. - delete_throwaway: when False, only empty/incognito sessions are deleted. + delete_throwaway: retained for caller compatibility. Ordinary sessions + with saved messages are always preserved, regardless of this flag. Returns a human-readable summary of what was done. """ @@ -71,7 +62,7 @@ async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bo db = SessionLocal() try: - # ── Phase 1: Delete empty/throwaway sessions ── + # ── Phase 1: Delete only empty and explicitly Incognito sessions ── deleted_empty = 0 deleted_throwaway = 0 @@ -93,47 +84,13 @@ async def run_auto_sort(owner: str, skip_llm: bool = False, delete_throwaway: bo if is_session_recently_active(row, now=cleanup_now): continue - msg_count = db.query(DbMsg.id).filter( + has_messages = db.query(DbMsg.id).filter( DbMsg.session_id == row.id - ).limit(_THROWAWAY_MAX_MESSAGES + 1).count() - should_delete = False - - if msg_count == 0: + ).first() is not None + if not has_messages: if is_fresh: continue - should_delete = True deleted_empty += 1 - elif delete_throwaway and msg_count <= _THROWAWAY_MAX_MESSAGES: - name = (row.name or "").strip().lower() - first_msg = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id, DbMsg.role == "user" - ).order_by(DbMsg.timestamp).first() - first_text = (first_msg[0] or "").strip().lower() if first_msg else "" - assistant_count = db.query(DbMsg.id).filter( - DbMsg.session_id == row.id, DbMsg.role == "assistant" - ).limit(1).count() - - if name in _THROWAWAY_NAMES or name.startswith("chat:") or first_text in _THROWAWAY_NAMES: - should_delete = True - deleted_throwaway += 1 - elif msg_count == 1 and assistant_count == 0: - should_delete = True - deleted_throwaway += 1 - elif msg_count <= 4 and first_text and len(first_text.split()) <= 8 and len(first_text) <= 80: - # Short trivial chats — e.g. "write hi to a friend" → "Hi!" - should_delete = True - deleted_throwaway += 1 - else: - # Aggressive: total message text under 250 chars combined = trivial - msg_rows = db.query(DbMsg.content).filter( - DbMsg.session_id == row.id - ).all() - total_chars = sum(len(m[0] or "") for m in msg_rows) - if total_chars <= 250: - should_delete = True - deleted_throwaway += 1 - - if should_delete: db.delete(row) if deleted_empty or deleted_throwaway: diff --git a/tests/test_chat_tidy_content_preservation.py b/tests/test_chat_tidy_content_preservation.py new file mode 100644 index 000000000..6c2260a98 --- /dev/null +++ b/tests/test_chat_tidy_content_preservation.py @@ -0,0 +1,148 @@ +"""Both chat tidy entry points retain saved conversations and still remove empties.""" +import asyncio +from datetime import datetime, timedelta, timezone + +import pytest +from fastapi import APIRouter, FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +import core.database as database +import core.session_manager as manager_module +import routes.session_routes as routes +import src.session_actions as actions + + +@pytest.fixture +def tidy_api(tmp_path, monkeypatch): + engine = create_engine( + f"sqlite:///{(tmp_path / 'tidy.db').as_posix()}", + connect_args={"check_same_thread": False}, + ) + database.Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, autoflush=False) + monkeypatch.setattr(database, "SessionLocal", factory) + monkeypatch.setattr(manager_module, "SessionLocal", factory) + monkeypatch.setattr(routes, "SessionLocal", factory) + monkeypatch.setattr(routes, "router", APIRouter(prefix="/api")) + monkeypatch.setattr(routes, "effective_user", lambda request: "alice") + manager = manager_module.SessionManager.__new__(manager_module.SessionManager) + manager.sessions = {} + manager.upload_handler = None + monkeypatch.setattr(manager, "get_sessions_for_user", lambda owner: {}) + app = FastAPI() + app.include_router(routes.setup_session_routes(manager, {})) + with TestClient(app) as client: + yield client, factory, manager + engine.dispose() + + +def run_tidy(client, entrypoint): + if entrypoint == "action": + return asyncio.run(actions.run_auto_sort("alice", skip_llm=True)) + response = client.post("/api/sessions/auto-sort?skip_llm=true") + assert response.status_code == 200, response.text + return response.json() + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +@pytest.mark.parametrize("title,prompt", [ + ("New chat", "Please analyze the following contract and retain all details. " * 20), + ("Contract analysis", "Review contract"), + ("Chat: analysis", "Explain these results and retain the full answer"), + ("Test", "Here is a large input that should be analyzed and retained"), +], ids=["generic-title", "short-prompt", "title-prefix", "test-title"]) +def test_tidy_preserves_nontrivial_saved_answer(tidy_api, title, prompt, entrypoint): + client, factory, manager = tidy_api + old = datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(hours=2) + answer = "Clause analysis and recommended changes.\n" * 2000 + with factory() as db: + db.add(database.Session( + id="valuable-chat", name=title, owner="alice", endpoint_url="", model="", + archived=False, is_important=False, message_count=2, + created_at=old, updated_at=old, last_accessed=old, last_message_at=old, + )) + db.commit() + db.add_all([ + database.ChatMessage(id="prompt", session_id="valuable-chat", role="user", + content=prompt, timestamp=old), + database.ChatMessage(id="answer", session_id="valuable-chat", role="assistant", + content=answer, timestamp=old + timedelta(seconds=1)), + ]) + db.commit() + run_tidy(client, entrypoint) + with factory() as db: + remaining = db.query(database.ChatMessage).filter_by(session_id="valuable-chat").count() + survives = db.get(database.Session, "valuable-chat") is not None + assert survives and remaining == 2, "Title/prompt heuristics must not erase saved answers" + with factory() as db: + assert db.get(database.ChatMessage, "answer").content == answer + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +@pytest.mark.parametrize("kind", ["unanswered", "greeting", "empty", "fresh-empty", "starred-empty", "archived", "other-owner", "incognito"]) +def test_tidy_cleanup_boundaries(tidy_api, entrypoint, kind): + client, factory, manager = tidy_api + now = datetime.now(timezone.utc).replace(tzinfo=None) + timestamp = now if kind == "fresh-empty" else now - timedelta(hours=2) + message = kind in {"unanswered", "greeting", "archived", "other-owner", "incognito"} + with factory() as db: + db.add(database.Session( + id="boundary-chat", name="Incognito" if kind == "incognito" else "New chat", + owner="bob" if kind == "other-owner" else "alice", endpoint_url="", model="", + archived=kind == "archived", is_important=kind == "starred-empty", + # Deliberately inaccurate metadata: cleanup must use message rows. + message_count=0 if message else 40, + created_at=timestamp, updated_at=timestamp, last_accessed=timestamp, last_message_at=timestamp, + )) + db.commit() + if message: + db.add(database.ChatMessage(id="boundary-message", session_id="boundary-chat", role="user", + content="hi" if kind == "greeting" else "Pending question " * 100, + timestamp=timestamp)) + db.commit() + run_tidy(client, entrypoint) + deleted = kind in {"empty", "incognito"} + with factory() as db: + assert (db.get(database.Session, "boundary-chat") is None) == deleted + assert db.query(database.ChatMessage).filter_by(session_id="boundary-chat").count() == int(message and not deleted) + + +@pytest.mark.parametrize("entrypoint", ["route", "action"]) +def test_tidy_keeps_folder_assignment_working(tidy_api, monkeypatch, entrypoint): + from types import SimpleNamespace + import src.llm_core as llm + import src.task_endpoint as endpoints + client, factory, manager = tidy_api + old = datetime.now(timezone.utc).replace(tzinfo=None) - timedelta(hours=2) + for sid in ["chat-one-long-id", "chat-two-long-id"]: + with factory() as db: + db.add(database.Session(id=sid, name="New chat", owner="alice", endpoint_url="", model="", + archived=False, created_at=old, updated_at=old, + last_accessed=old, last_message_at=old)) + db.commit() + db.add(database.ChatMessage(id=f"{sid}-message", session_id=sid, role="user", + content="Saved question", timestamp=old)) + db.commit() + manager.sessions[sid] = SimpleNamespace(id=sid, name="New chat", archived=False, + updated_at=old.isoformat(), created_at=old.isoformat()) + monkeypatch.setattr(manager, "get_sessions_for_user", lambda owner: manager.sessions) + monkeypatch.setattr(endpoints, "resolve_task_endpoint", lambda **kwargs: ("http://model.invalid", "test", {})) + response_text = '{"folders":{"Projects":["chat-one","chat-two"]}}' + monkeypatch.setattr(llm, "llm_call", lambda *args, **kwargs: response_text) + + async def async_response(*args, **kwargs): + return response_text + + monkeypatch.setattr(llm, "llm_call_async", async_response) + if entrypoint == "action": + result = asyncio.run(actions.run_auto_sort("alice")) + assert "Sorted 2 sessions" in result + else: + response = client.post("/api/sessions/auto-sort") + assert response.status_code == 200, response.text + assert response.json()["updated"] == 2 + with factory() as db: + assert db.query(database.ChatMessage).count() == 2 + assert {row.folder for row in db.query(database.Session).all()} == {"Projects"} diff --git a/tests/test_session_actions_cleanup.py b/tests/test_session_actions_cleanup.py index 221713d33..be89a29de 100644 --- a/tests/test_session_actions_cleanup.py +++ b/tests/test_session_actions_cleanup.py @@ -128,7 +128,7 @@ def test_auto_sort_keeps_fresh_session_while_first_response_is_pending(monkeypat db.close() -def test_auto_sort_still_deletes_old_throwaway_sessions(monkeypatch): +def test_auto_sort_keeps_old_nonempty_sessions_even_with_throwaway_title(monkeypatch): session_factory = _make_session_factory() _install_session_factory(monkeypatch, session_factory) @@ -160,7 +160,8 @@ def test_auto_sort_still_deletes_old_throwaway_sessions(monkeypatch): db = session_factory() try: - assert db.query(DbSession).filter(DbSession.id == sid).first() is None - assert "Cleaned 1 sessions" in result + assert db.query(DbSession).filter(DbSession.id == sid).first() is not None + assert db.query(DbMessage).filter(DbMessage.session_id == sid).count() == 1 + assert "Cleaned 0 sessions" in result finally: db.close() diff --git a/tests/test_session_list_owner_scope.py b/tests/test_session_list_owner_scope.py index fc973bd53..aacde8d29 100644 --- a/tests/test_session_list_owner_scope.py +++ b/tests/test_session_list_owner_scope.py @@ -19,6 +19,7 @@ from core.database import ChatMessage as DbMessage from core.database import Session as DbSession _TMPDB = tempfile.NamedTemporaryFile(suffix=".db", delete=False) +_TMPDB.close() # SQLite owns its connections; release the Windows file handle. _ENGINE = create_engine( f"sqlite:///{_TMPDB.name}", connect_args={"check_same_thread": False}, @@ -230,12 +231,14 @@ def test_patch_session_updates_persisted_cwd(monkeypatch): def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(monkeypatch): import routes.session_routes as sr + from fastapi import APIRouter from unittest.mock import MagicMock _stub_multipart_if_missing(monkeypatch) monkeypatch.setenv("AUTH_ENABLED", "false") monkeypatch.setattr(sr, "SessionLocal", _TS) monkeypatch.setattr(sr, "effective_user", lambda request: None) + monkeypatch.setattr(sr, "router", APIRouter(prefix="/api")) sid = str(uuid.uuid4()) old_time = cdb.utcnow_naive() - timedelta(hours=2) @@ -250,19 +253,12 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon endpoint_url="http://localhost", model="gpt-4", archived=False, - message_count=1, + message_count=0, created_at=old_time, updated_at=old_time, last_message_at=old_time, last_accessed=old_time, )) - db.add(DbMessage( - id="m-" + uuid.uuid4().hex, - session_id=sid, - role="user", - content="hi", - timestamp=old_time, - )) db.commit() finally: db.close() @@ -270,6 +266,13 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon session = MagicMock(id=sid, name="New chat", model="gpt-4", endpoint_url="http://localhost", rag=False, archived=False) sm = MagicMock() sm.get_sessions_for_user.return_value = {sid: session} + + def delete_session(session_id): + with _TS() as db: + db.query(DbSession).filter(DbSession.id == session_id).delete() + db.commit() + + sm.delete_session.side_effect = delete_session router = sr.setup_session_routes(sm, {}) endpoint = next(r.endpoint for r in router.routes if getattr(r, "path", "") == "/api/sessions/auto-sort" @@ -277,7 +280,8 @@ def test_auto_sort_skip_llm_cleans_owner_stamped_sessions_when_auth_disabled(mon result = endpoint(request=MagicMock(), skip_llm=True) - assert result["deleted_throwaway"] == 1 + assert result["deleted_empty"] == 1 + assert result["deleted_throwaway"] == 0 db = _TS() try: assert db.query(DbSession).filter(DbSession.id == sid).first() is None From 7fd4e0e9bb1da327e284fa1aa0af99c744fde1cd Mon Sep 17 00:00:00 2001 From: PR #6029 Reviewer Date: Tue, 20 Oct 2026 11:39:51 +0100 Subject: [PATCH 04/13] fix(ui): harden iOS safe-area and viewport regressions --- static/css/17-research.css | 9 +- static/index.html | 7 - static/js/init.js | 2 - static/js/windowResize.js | 5 +- tests/e2e/ios-layout.config.cjs | 58 ++++++ .../e2e/ios-layout/postmerge-layout.spec.cjs | 181 ++++++++++++++++++ 6 files changed, 248 insertions(+), 14 deletions(-) create mode 100644 tests/e2e/ios-layout.config.cjs create mode 100644 tests/e2e/ios-layout/postmerge-layout.spec.cjs diff --git a/static/css/17-research.css b/static/css/17-research.css index c4de5cb75..2c43aa2c8 100644 --- a/static/css/17-research.css +++ b/static/css/17-research.css @@ -2690,11 +2690,14 @@ body.theme-frosted .modal { max-height: 95dvh !important; } + /* Notes and Documents must respect the same viewport and safe-area + top as other mobile full-height sheets. Keep these authoritative rules + consistent with 05-documents.css and 14-email.css. */ body.doc-view .doc-editor-pane, body.notes-view .notes-pane { - inset: auto 0 0 !important; - height: 95dvh !important; - max-height: 95dvh !important; + inset: var(--sheet-top) 0 0 !important; + height: calc(var(--app-height) - var(--sheet-top)) !important; + max-height: calc(var(--app-height) - var(--sheet-top)) !important; } } diff --git a/static/index.html b/static/index.html index 89836d27a..5fc386a5c 100644 --- a/static/index.html +++ b/static/index.html @@ -16,13 +16,6 @@