diff --git a/studio/install_node_prebuilt.py b/studio/install_node_prebuilt.py index be2802efd8..d370a108e3 100644 --- a/studio/install_node_prebuilt.py +++ b/studio/install_node_prebuilt.py @@ -16,6 +16,8 @@ Archives are verified against sha256 digests pinned in ``node_prebuilt_pins.json Mirrors ``install_llama_prebuilt.py`` so the setup scripts drive it the same way. Exit codes: 0 success, 1 error, 2 fallback, 3 busy, 4 access denied. A re-run that already matches logs "already matches" and returns 0 without downloading (the scripts grep it). +A failed update that keeps a usable install logs "keeping existing isolated Node" and +also returns 0 (the scripts grep that too). """ from __future__ import annotations @@ -1013,11 +1015,40 @@ def existing_install_usable(install_dir: Path, host: HostInfo) -> bool: return npm_major is not None and npm_major >= NPM_MIN_MAJOR +# Retried (scanners raise it right after extraction), but unreadable ACLs raise it too (#9928). +_ERROR_ACCESS_DENIED = 5 + + +def _access_denied_recovery_lines(paths: tuple[tuple[Path, bool], ...]) -> list[str]: + """Repair commands for a rename still failing with ``ERROR_ACCESS_DENIED``. + + One command per line: joined, takeown swallows the rest as arguments. + """ + lines = [ + "if this was not a scanner, the ACLs on one of these rename paths may be " + "unreadable -- even icacls/Get-Acl report access denied in that state", + "from an elevated PowerShell, restore access for each affected path, " + "then run the install again:", + ] + # Non-recursive for the parent: /R would rewrite sibling installs. + for path, recursive in dict.fromkeys(paths): + takeown_flags = " /R /D Y" if recursive else "" + icacls_flags = " /T /C" if recursive else "" + lines.extend( + ( + f' takeown /F "{path}"{takeown_flags}', + f' icacls "{path}" /reset{icacls_flags}', + ) + ) + return lines + + def _replace_with_retry( src: Path, dst: Path, *, attempts: int = 8, + access_denied_paths: tuple[tuple[Path, bool], ...] | None = None, ) -> None: """os.replace, retried against transient Windows sharing violations. @@ -1026,6 +1057,10 @@ def _replace_with_retry( a fresh install, with no existing directory to conflict with). Handles clear in a second or two, so a bounded backoff turns the failure into a pause; other errors raise immediately rather than stalling on a real problem. + + An exhausted WinError 5 may be an ACL fault, not a scanner (#9928): with + ``access_denied_paths`` the repair lines ride on the error, printed only if an existing + Node is kept (otherwise main() exits 4 and setup.ps1 prints its own, #10533). """ delay = 0.25 for attempt in range(attempts): @@ -1033,13 +1068,20 @@ def _replace_with_retry( os.replace(src, dst) return except OSError as exc: - transient = os.name == "nt" and getattr(exc, "winerror", None) in (5, 32, 145) + winerror = getattr(exc, "winerror", None) + transient = os.name == "nt" and winerror in (_ERROR_ACCESS_DENIED, 32, 145) if not transient or attempt == attempts - 1: + if transient and winerror == _ERROR_ACCESS_DENIED and access_denied_paths: + log(f"rename still blocked (5) after {attempts} attempts") + exc._unsloth_acl_recovery_lines = _access_denied_recovery_lines( + access_denied_paths + ) raise - log( - f"rename blocked ({exc.winerror}), retrying in {delay:.2f}s " - f"-- a scanner is likely still holding the extracted files" - ) + if winerror == _ERROR_ACCESS_DENIED: + cause = "a scanner may still hold the files, or the ACLs are unreadable" + else: + cause = "a scanner is likely still holding the extracted files" + log(f"rename blocked ({winerror}), retrying in {delay:.2f}s -- {cause}") time.sleep(delay) delay = min(delay * 2, 4.0) @@ -1050,16 +1092,28 @@ def _swap_into_place(extracted_root: Path, install_dir: Path) -> None: backup: Path | None = None if install_dir.exists(): backup = install_dir.parent / f".{install_dir.name}.old-{os.getpid()}" - _replace_with_retry(install_dir, backup) + _replace_with_retry( + install_dir, + backup, + access_denied_paths = ((install_dir, True), (install_dir.parent, False)), + ) try: - _replace_with_retry(extracted_root, install_dir) + _replace_with_retry( + extracted_root, + install_dir, + access_denied_paths = ((extracted_root, True), (install_dir.parent, False)), + ) except OSError: # The forward rename retries ~16s, ample time for a scanner to grab the backup too. # A plain os.replace would raise over the original error and leave no install_dir at all, so the rollback gets # the same backoff and never masks it. if backup is not None and not install_dir.exists(): try: - _replace_with_retry(backup, install_dir) + _replace_with_retry( + backup, + install_dir, + access_denied_paths = ((backup, True), (install_dir.parent, False)), + ) except OSError as rollback_exc: log(f"could not restore the previous Node install from {backup}: {rollback_exc}") raise @@ -1185,7 +1239,15 @@ def install_prebuilt(install_dir: Path, *, channel: str, min_major: int, force: and meta.get("sha256") != pin ) if not force and not pin_mismatch and existing_install_usable(install_dir, host): - log(f"Node download failed ({exc}); keeping existing isolated Node") + recovery = getattr(exc, "_unsloth_acl_recovery_lines", None) + if recovery: + for line in recovery: + log(line) + log( + f"existing Node could not be replaced ({exc}); keeping existing isolated Node" + ) + else: + log(f"Node download failed ({exc}); keeping existing isolated Node") return EXIT_SUCCESS raise diff --git a/studio/setup.ps1 b/studio/setup.ps1 index cf63e07ef1..200289f1e2 100644 --- a/studio/setup.ps1 +++ b/studio/setup.ps1 @@ -5722,6 +5722,12 @@ if ($NeedNodeForSetup) { Write-StudioLine "[ERROR] Could not install an isolated Node automatically." -ForegroundColor Red Write-StudioLine " Install Node >= 20.19 (with npm >= 11) from https://nodejs.org/ and re-run, or check your network." -ForegroundColor Yellow Exit-SetupFailure "Could not install an isolated Node runtime" + } elseif ($nodeOut -match "keeping existing isolated Node") { + # Exit 0 also covers a failed update that kept a working Node; relay any repair lines. + if ($nodeOut -match 'takeown /F') { + Write-StudioLine $nodeOut -ForegroundColor DarkGray + } + step "node" "update not applied, existing isolated Node kept" "Yellow" } # Same condition as the guard above, or the tree this run just created under a master # root stays unmarked and the uninstaller declines to remove it as somebody else's. @@ -9766,12 +9772,12 @@ if ($LocalLlamaCppSrc) { Remove-Item -Recurse -Force -LiteralPath $LlamaCppDir -ErrorAction SilentlyContinue # A locked/in-use tree can silently survive removal (SilentlyContinue # masks it). Don't then junction/copy over a half-present dir; mirror the - # prebuilt path's active-process handling and stop with a clear message. - # Denied counts as surviving: unreadable is not gone. + # prebuilt path's blocked-install handling and stop with a clear message. + # Denied counts as surviving (unreadable is not gone), so name no single cause (#9928). if ((Get-PathState -Path $LlamaCppDir) -ne "Absent") { - step "llama.cpp" "install blocked by active llama.cpp process" "Yellow" - substep "Close Unsloth or other llama.cpp users and retry" "Yellow" - Exit-SetupFailure "llama.cpp install is blocked by an active llama.cpp process" 3 + step "llama.cpp" "install blocked; existing install could not be replaced" "Yellow" + substep "Close Unsloth or other llama.cpp users, or repair the ACLs on $LlamaCppDir, then retry" "Yellow" + Exit-SetupFailure "llama.cpp install is blocked; the existing install could not be replaced" 3 } } cmd /c "mklink /J `"$LlamaCppDir`" `"$ResolvedLocal`"" 2>&1 | Out-Null @@ -9940,13 +9946,18 @@ if ($LocalLlamaCppLinked) { substep $installedRelease } } elseif ($prebuiltExit -eq 3) { - step "llama.cpp" "install blocked by active llama.cpp process" "Yellow" + # WinError 5 is both an in-use file and an unreadable ACL: name ACLs only if repair lines were printed. + step "llama.cpp" "install blocked; existing install could not be replaced" "Yellow" Write-LlamaFailureLog -Output $prebuiltOutput if (Test-Path -LiteralPath $LlamaCppDir) { substep "Existing install was restored" "Yellow" } - substep "Close Unsloth or other llama.cpp users and retry" "Yellow" - Exit-SetupFailure "llama.cpp install is blocked by an active llama.cpp process" 3 + if ($prebuiltOutput -match 'takeown /F') { + substep "Close Unsloth or other llama.cpp users, or repair the ACLs named above, then retry" "Yellow" + } else { + substep "Close Unsloth or other llama.cpp users and retry" "Yellow" + } + Exit-SetupFailure "llama.cpp install is blocked; the existing install could not be replaced" 3 } elseif ($prebuiltExit -eq 4) { step "llama.cpp" "not enough disk space to install llama.cpp" "Yellow" Write-LlamaFailureLog -Output $prebuiltOutput diff --git a/studio/setup.sh b/studio/setup.sh index e7f14c6f23..028b389d82 100755 --- a/studio/setup.sh +++ b/studio/setup.sh @@ -2262,6 +2262,12 @@ elif [ "$NODE_SOURCE" = bundled ]; then sed 's/^/ | /' "$_NODE_LOG" >&2; rm -f "$_NODE_LOG" substep "install Node >= 20.19 (with npm >= 11) yourself and re-run, or check your network" setup_fail 1 "Could not install an isolated Node runtime" + elif grep -Fq "keeping existing isolated Node" "$_NODE_LOG"; then + # Exit 0 also covers a failed update that kept a working Node; relay any repair lines. + if grep -Fq 'takeown /F' "$_NODE_LOG"; then + sed 's/^/ | /' "$_NODE_LOG" >&2 + fi + step "node" "update not applied, existing isolated Node kept" "$C_WARN" fi grep -Fq "already matches" "$_NODE_LOG" && verbose_substep "isolated Node already up to date" rm -f "$_NODE_LOG" diff --git a/tests/sh/test_setup_node_kept_existing.sh b/tests/sh/test_setup_node_kept_existing.sh new file mode 100644 index 0000000000..6f272d82e0 --- /dev/null +++ b/tests/sh/test_setup_node_kept_existing.sh @@ -0,0 +1,102 @@ +#!/bin/bash +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 +# setup.sh's Node exit handling (extracted from the real script) must report a kept Node +# and relay its takeown/icacls lines on exit 0 (#9928). +set -uo pipefail + +HERE="$(CDPATH= cd -P -- "$(dirname "$0")" && pwd -P)" +SETUP="$HERE/../../studio/setup.sh" +fails=0 +check() { # name expected-substring (or !forbidden-substring) actual + case "$2" in + '!'*) + case "$3" in + *"${2#!}"*) printf ' FAIL %s : did not expect [%s] in:\n%s\n' "$1" "${2#!}" "$3"; fails=$((fails+1)) ;; + *) printf ' PASS %s\n' "$1" ;; + esac ;; + *) + case "$3" in + *"$2"*) printf ' PASS %s\n' "$1" ;; + *) printf ' FAIL %s : expected [%s] in:\n%s\n' "$1" "$2" "$3"; fails=$((fails+1)) ;; + esac ;; + esac +} + +# From the installer call to the log cleanup after the "already matches" check. +BLOCK="$(awk ' + /^ _NODE_LOG="\$\(mktemp\)"$/ {grab = 1} + grab {print} + grab && /^ rm -f "\$_NODE_LOG"$/ {exit} +' "$SETUP")" +case "$BLOCK" in + *'install_node_prebuilt.py'*'_NODE_STATUS'*'already matches'*'rm -f "$_NODE_LOG"') : ;; + *) echo "FAIL: the Node exit-handling block could not be extracted from setup.sh"; exit 1 ;; +esac + +T="$(mktemp -d)" +trap 'rm -rf "$T"' EXIT +cat > "$T/fake_installer" <<'EOF' +#!/bin/bash +cat "$FAKE_OUTPUT" +exit "$FAKE_STATUS" +EOF +chmod +x "$T/fake_installer" + +run_block() { # exit-status installer-output + printf '%s\n' "$2" > "$T/output" + env -i PATH="$PATH" TMPDIR="$T" FAKE_OUTPUT="$T/output" FAKE_STATUS="$1" \ + FAKE_INSTALLER="$T/fake_installer" BLOCK="$BLOCK" \ + bash -c ' + set -euo pipefail + C_OK=ok C_WARN=warn C_ERR=err C_DIM= C_RST= + step() { printf "STEP %s | %s | %s\n" "$1" "$2" "${3:-ok}"; } + substep() { printf "SUBSTEP %s\n" "$1"; } + verbose_substep() { printf "VERBOSE %s\n" "$1"; } + setup_fail() { printf "SETUP_FAIL %s | %s\n" "$1" "$2"; exit "$1"; } + _is_verbose() { return 1; } + _NODE_PY="$FAKE_INSTALLER" + SCRIPT_DIR=/nonexistent + NODE_DIR=/nonexistent/node + eval "$BLOCK" + echo "CONTINUED" + ' 2>&1 +} + +DENIED='[node-prebuilt] rename still blocked (5) after 8 attempts +[node-prebuilt] takeown /F "C:\Users\u\.unsloth\node" /R /D Y +[node-prebuilt] icacls "C:\Users\u\.unsloth\node" /reset /T /C +[node-prebuilt] existing Node could not be replaced ([WinError 5] Access is denied); keeping existing isolated Node' + +echo "" +echo "=== exit 0 after a denied rename kept the running Node ===" +out="$(run_block 0 "$DENIED")" +check "reported as a kept install, as a warning" "STEP node | update not applied, existing isolated Node kept | warn" "$out" +check "not llama.cpp's keep text, which setup counts as exactly two arms" "!existing prebuilt kept" "$out" +check "the takeown/icacls lines reach the user" ' | [node-prebuilt] takeown /F' "$out" +check "setup carries on" "CONTINUED" "$out" +check "setup does not fail" "!SETUP_FAIL" "$out" + +echo "" +echo "=== exit 0 after a failed download kept the running Node ===" +out="$(run_block 0 '[node-prebuilt] Node download failed (nodejs.org unreachable); keeping existing isolated Node')" +check "reported as a kept install, as a warning" "STEP node | update not applied, existing isolated Node kept | warn" "$out" +check "no repair lines, so the installer output is not relayed" "! | " "$out" +check "setup carries on" "CONTINUED" "$out" + +echo "" +echo "=== exit 0 after a fresh install ===" +out="$(run_block 0 '[node-prebuilt] installed isolated Node v24.18.0 (npm 11.x) at /x/node')" +check "no kept-install warning" "!existing isolated Node kept" "$out" +check "setup carries on" "CONTINUED" "$out" + +echo "" +echo "=== a non-zero exit still fails setup, whatever the output says ===" +out="$(run_block 1 "$DENIED")" +check "exit 1 fails setup" "SETUP_FAIL 1" "$out" +check "a failed install is not reported as kept" "!existing isolated Node kept" "$out" +out="$(run_block 3 '[node-prebuilt] another install holds the lock')" +check "exit 3 fails setup with 3" "SETUP_FAIL 3" "$out" + +if [ "$fails" -ne 0 ]; then echo "$fails check(s) failed"; exit 1; fi +echo "All checks passed" diff --git a/tests/studio/install/test_install_node_prebuilt_logic.py b/tests/studio/install/test_install_node_prebuilt_logic.py index e58196fa70..1b6b46e896 100644 --- a/tests/studio/install/test_install_node_prebuilt_logic.py +++ b/tests/studio/install/test_install_node_prebuilt_logic.py @@ -530,6 +530,127 @@ def test_install_prebuilt_reraises_download_failure_without_existing(tmp_path: P M.install_prebuilt(install_dir, channel = "lts", min_major = 24, force = False) +def _swap_stays_denied( + tmp_path: Path, + monkeypatch, + *, + recorded: str, + runs: bool, + real_swap: bool = False, +) -> Path: + # real_swap runs the real _swap_into_place against a refused rename. + install_dir = tmp_path / "node" + install_dir.mkdir() + M.write_metadata(install_dir, version = recorded, asset = "old", sha256 = "old") + monkeypatch.setattr(M, "detect_host", lambda: _host("linux", "x64")) + monkeypatch.setattr(M, "installed_node_version", lambda d, h: recorded if runs else None) + monkeypatch.setattr(M, "installed_npm_major", lambda d, h: 11) + monkeypatch.setattr( + M, "download_file_verified", lambda url, path, **kw: path.write_bytes(b"zip") + ) + + def fake_extract(_archive, extract_dir): + (extract_dir / "node-v24").mkdir(parents = True) + + def reported_acl_denial(_extracted, _install_dir): + exc = _oserror(5) + # Literal, so the parity cases also run against a module without the helper. + exc._unsloth_acl_recovery_lines = [f' takeown /F "{install_dir}" /R /D Y'] + raise exc + + monkeypatch.setattr(M, "extract_archive", fake_extract) + monkeypatch.setattr(M, "_ensure_npm_floor", lambda d, h: None) + if not real_swap: + monkeypatch.setattr(M, "_swap_into_place", reported_acl_denial) + return install_dir + + real_os = M.os + + class _WindowsRenames: + # Module-local only: a real os.name = "nt" makes Path() a WindowsPath, unusable on Linux. + name = "nt" + + def __getattr__(self, attr): + return getattr(real_os, attr) + + @staticmethod + def replace(src, dst): + if Path(src).is_dir(): + exc = PermissionError(13, "Access is denied", str(src), None, str(dst)) + exc.winerror = 5 + raise exc + return real_os.replace(src, dst) + + monkeypatch.setattr(M, "os", _WindowsRenames()) + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + return install_dir + + +def test_install_prebuilt_keeps_existing_when_a_reported_denial_blocks_the_swap( + tmp_path: Path, monkeypatch, capsys +): + install_dir = _swap_stays_denied(tmp_path, monkeypatch, recorded = "24.9.0", runs = True) + + rc = M.install_prebuilt(install_dir, channel = "pinned", min_major = 24, force = False) + + assert rc == M.EXIT_SUCCESS + output = "".join(capsys.readouterr()) + assert "could not be replaced" in output + assert "keeping existing isolated Node" in output + assert "download failed" not in output + + +@pytest.mark.parametrize( + "runs, force, recorded_is_pinned", + [ + pytest.param(False, False, False, id = "node-does-not-run"), + pytest.param(True, True, False, id = "force"), + pytest.param(True, False, True, id = "recorded-digest-is-not-the-pin"), + ], +) +def test_install_prebuilt_reraises_a_reported_denial_it_cannot_keep_existing_through( + tmp_path: Path, monkeypatch, runs, force, recorded_is_pinned +): + recorded = M.pinned_default_version(M.load_pins()) if recorded_is_pinned else "24.9.0" + install_dir = _swap_stays_denied(tmp_path, monkeypatch, recorded = recorded, runs = runs) + + with pytest.raises(OSError) as excinfo: + M.install_prebuilt(install_dir, channel = "pinned", min_major = 24, force = force) + + assert excinfo.value.winerror == 5 + + +def test_a_denial_that_cannot_keep_node_leaves_the_repair_to_setup( + tmp_path: Path, monkeypatch, capsys +): + # Exit 4: setup.ps1 prints its own repair (#10533); ours too would show two sets. + install_dir = _swap_stays_denied( + tmp_path, monkeypatch, recorded = "24.9.0", runs = False, real_swap = True + ) + + rc = M.main(["--install-dir", str(install_dir)]) + + output = capsys.readouterr().out + assert rc == M.EXIT_DENIED + assert "rename still blocked (5)" in output + assert f"{M.DENIED_SCOPE_MARKER}{M.DENIED_SCOPE_INSTALL_DIR}" in output + assert "takeown" not in output, output + + +def test_a_kept_node_prints_the_repair_setup_relays(tmp_path: Path, monkeypatch, capsys): + install_dir = _swap_stays_denied( + tmp_path, monkeypatch, recorded = "24.9.0", runs = True, real_swap = True + ) + + rc = M.main(["--install-dir", str(install_dir)]) + + output = capsys.readouterr().out + assert rc == M.EXIT_SUCCESS + assert f'takeown /F "{install_dir}" /R /D Y' in output + assert f'takeown /F "{install_dir.parent}"' in output + assert output.index("takeown") < output.index("existing Node could not be replaced") + + # ── Isolation invariant: the installer only writes inside its own install_dir ── def test_run_node_pins_npm_prefix_to_install_dir(tmp_path: Path, monkeypatch): # Every node/npm call the installer makes redirects npm's global prefix into @@ -896,6 +1017,80 @@ def test_replace_gives_up_and_reports_the_real_error(monkeypatch, tmp_path): assert excinfo.value.winerror == 5 +def test_replace_names_acl_recovery_when_access_denied_persists(monkeypatch, tmp_path, capsys): + """A WinError 5 that outlasts the budget names the ACL recovery (#9928).""" + source = tmp_path / "node" + source.mkdir() + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + monkeypatch.setattr(M.os, "replace", lambda s, d: (_ for _ in ()).throw(_oserror(5))) + + with pytest.raises(OSError) as excinfo: + M._replace_with_retry( + source, tmp_path / "node.old", attempts = 3, access_denied_paths = ((source, True),) + ) + + recovery = getattr(excinfo.value, "_unsloth_acl_recovery_lines", []) + assert any("takeown" in line for line in recovery), recovery + assert any("icacls" in line for line in recovery), recovery + assert any("elevated PowerShell" in line for line in recovery), recovery + assert any(str(source) in line for line in recovery), recovery + assert not any("takeown" in line and "icacls" in line for line in recovery), recovery + assert "takeown" not in "".join(capsys.readouterr()) + + +def test_a_denied_marker_write_offers_no_repair_for_its_temp_file(monkeypatch, tmp_path, capsys): + # The source is a temp file about to be removed: nothing to repair. + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + monkeypatch.setattr(M.os, "replace", lambda s, d: (_ for _ in ()).throw(_oserror(5))) + marker = M.metadata_path(tmp_path) + temp = marker.with_name(marker.name + ".tmp-denied") + temp.write_text("{}", encoding = "utf-8") + + with pytest.raises(OSError) as excinfo: + M.atomic_replace_from_tempfile(temp, marker) + + assert excinfo.value.winerror == 5 + assert not getattr(excinfo.value, "_unsloth_acl_recovery_lines", None) + assert "takeown" not in "".join(capsys.readouterr()) + + +def test_replace_does_not_blame_a_scanner_for_access_denied(monkeypatch, tmp_path, capsys): + """The per-retry line for a 5 offers both causes rather than asserting the scanner.""" + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + monkeypatch.setattr(M.os, "replace", lambda s, d: (_ for _ in ()).throw(_oserror(5))) + + with pytest.raises(OSError): + M._replace_with_retry(tmp_path / "src", tmp_path / "dst", attempts = 2) + + retry_lines = [ + line for line in "".join(capsys.readouterr()).splitlines() if "retrying in" in line + ] + assert retry_lines, "the retry itself must still be logged" + for line in retry_lines: + assert "is likely still holding" not in line, line + assert "ACLs" in line, line + + +def test_replace_keeps_the_scanner_message_for_a_sharing_violation(monkeypatch, tmp_path, capsys): + """WinError 32 is unambiguous, so its wording is untouched. + + Parity guard: passes with and without the #9928 change. + """ + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + monkeypatch.setattr(M.os, "replace", lambda s, d: (_ for _ in ()).throw(_oserror(32))) + + with pytest.raises(OSError): + M._replace_with_retry(tmp_path / "src", tmp_path / "dst", attempts = 2) + + output = "".join(capsys.readouterr()) + assert "a scanner is likely still holding the extracted files" in output + assert "takeown" not in output + + def test_replace_does_not_retry_a_genuine_error(monkeypatch, tmp_path): monkeypatch.setattr(M.os, "name", "nt") monkeypatch.setattr(M.time, "sleep", lambda _s: None) @@ -1501,3 +1696,53 @@ def test_a_tree_replaced_under_the_lock_is_still_not_a_match(tmp_path, monkeypat is False ) assert recorded == [], "a record was written over another installer's tree" + + +def test_swap_into_place_reports_the_managed_parent_on_destination_denial( + monkeypatch, tmp_path, capsys +): + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + extracted = tmp_path / "staging" / "node-v24" + extracted.mkdir(parents = True) + install_dir = tmp_path / "managed" / "node" + + def destination_denied(src, dst): + assert src == extracted + assert dst == install_dir + raise _oserror(5) + + monkeypatch.setattr(M.os, "replace", destination_denied) + with pytest.raises(OSError) as excinfo: + M._swap_into_place(extracted, install_dir) + + recovery = excinfo.value._unsloth_acl_recovery_lines + assert any(f'takeown /F "{extracted}"' in line for line in recovery), recovery + assert any(f'takeown /F "{install_dir.parent}"' in line for line in recovery), recovery + assert any("elevated PowerShell" in line for line in recovery), recovery + + +def test_swap_into_place_reports_the_managed_parent_when_aside_is_denied( + monkeypatch, tmp_path, capsys +): + monkeypatch.setattr(M.os, "name", "nt") + monkeypatch.setattr(M.time, "sleep", lambda _s: None) + extracted = tmp_path / "staging" / "node-v24" + extracted.mkdir(parents = True) + install_dir = tmp_path / "managed" / "node" + install_dir.mkdir(parents = True) + + def aside_denied(src, dst): + assert src == install_dir + assert dst.parent == install_dir.parent + raise _oserror(5) + + monkeypatch.setattr(M.os, "replace", aside_denied) + with pytest.raises(OSError) as excinfo: + M._swap_into_place(extracted, install_dir) + + recovery = excinfo.value._unsloth_acl_recovery_lines + assert f' takeown /F "{install_dir}" /R /D Y' in recovery + parent_takeown = f'takeown /F "{install_dir.parent}"' + assert any(parent_takeown in line for line in recovery), recovery + assert not any(parent_takeown in line and " /R " in line for line in recovery), recovery diff --git a/tests/studio_setup_ps1/Studio.Setup.BlockedInstallCause.Tests.ps1 b/tests/studio_setup_ps1/Studio.Setup.BlockedInstallCause.Tests.ps1 new file mode 100644 index 0000000000..fdcd441192 --- /dev/null +++ b/tests/studio_setup_ps1/Studio.Setup.BlockedInstallCause.Tests.ps1 @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. +<# + Source scan of setup.ps1's "install blocked" reporting (#9928): llama.cpp exit 3 + must not name a process (WinError 5 is also an unreadable ACL), and a Node kept + on exit 0 must relay the installer's repair lines. +#> + +BeforeAll { + $candidates = @( + $env:SETUP_PS1_PATH, + (Join-Path $PSScriptRoot '..\..\studio\setup.ps1') + ) | Where-Object { $_ } + $script:SetupPs1 = $candidates | Where-Object { Test-Path -LiteralPath $_ } | Select-Object -First 1 + if (-not $script:SetupPs1) { throw "Could not locate studio/setup.ps1 (set SETUP_PS1_PATH)." } + $script:SetupText = Get-Content -Raw -LiteralPath $script:SetupPs1 + $script:NodeInstaller = Get-Content -Raw -LiteralPath (Join-Path (Split-Path -Parent $script:SetupPs1) 'install_node_prebuilt.py') + + # These are inline installer flow, not functions, so Get-FunctionSource cannot + # reach them; slice from the marker to the first close at the marker's depth. + function Get-BlockSource { + param( + [Parameter(Mandatory)][string]$Path, + [Parameter(Mandatory)][string]$Marker, + [Parameter(Mandatory)][string]$ClosePattern + ) + $lines = @(Get-Content -LiteralPath $Path) + $start = -1 + for ($i = 0; $i -lt $lines.Count; $i++) { + if ($lines[$i] -match [regex]::Escape($Marker)) { $start = $i; break } + } + if ($start -lt 0) { return $null } + $indent = ($lines[$start] -replace '\S.*$', '').Length + for ($j = $start + 1; $j -lt $lines.Count; $j++) { + $depth = ($lines[$j] -replace '\S.*$', '').Length + if ($lines[$j] -match $ClosePattern -and $depth -le $indent) { + return ($lines[$start..($j - 1)] -join "`n") + } + } + return $null + } + + $script:LlamaBusy = Get-BlockSource -Path $script:SetupPs1 ` + -Marker '$prebuiltExit -eq 3' -ClosePattern '^\s*\}\s*(elseif|else)\b' + $script:WhisperBusy = Get-BlockSource -Path $script:SetupPs1 ` + -Marker '$whisperExit -eq 3' -ClosePattern '^\s*\}\s*(elseif|else)\b' + $script:LocalLinkBusy = Get-BlockSource -Path $script:SetupPs1 ` + -Marker '(Get-PathState -Path $LlamaCppDir) -ne "Absent"' -ClosePattern '^\s*\}\s*$' + # The whole if/elseif chain on the Node installer's exit code, and nothing after it. Anchored on the chain, + # not on reading $LASTEXITCODE: the mirror retry (#11786) sits between the two and closes first. + $script:NodeExit = Get-BlockSource -Path $script:SetupPs1 ` + -Marker '$nodeExit -eq 3' -ClosePattern '^\s*\}\s*$' +} + +Describe 'no llama.cpp blocked-install message names a cause setup cannot determine' { + It 'the process headline appears nowhere in setup.ps1' { + $script:SetupText | Should -Not -Match 'install blocked by active llama\.cpp process' + } + + It 'the process failure message appears nowhere in setup.ps1' { + $script:SetupText | Should -Not -Match 'blocked by an active llama\.cpp process' + } + + It 'every blocked-install site offers ACL repair' { + ([regex]::Matches($script:SetupText, 'repair the ACLs')).Count | + Should -BeGreaterOrEqual 2 + } +} + +Describe 'the prebuilt exit-3 branch' { + It 'is present in setup.ps1' { + $script:LlamaBusy | Should -Not -BeNullOrEmpty + } + + It 'offers ACL repair alongside closing other users' { + $script:LlamaBusy | Should -Match 'repair the ACLs' + } + + It 'only names ACL repair when the helper emitted recovery guidance' { + $script:LlamaBusy | Should -Match '\$prebuiltOutput -match [''"]takeown /F[''"]' + } + + It 'keeps a close-users-only fallback without recovery guidance' { + $script:LlamaBusy | Should -Match '(?s)else\s*\{.+Close Unsloth or other llama\.cpp users and retry' + } + + It 'still terminates with exit code 3' { + $script:LlamaBusy | Should -Match 'Exit-SetupFailure .+ 3' + } + + It 'still prints the installer output carrying the takeown/icacls commands' { + $script:LlamaBusy | Should -Match 'Write-LlamaFailureLog -Output \$prebuiltOutput' + } + + It 'still reports that the previous install was restored' { + $script:LlamaBusy | Should -Match 'Existing install was restored' + } + + It 'still tells the user to close other llama.cpp users' { + $script:LlamaBusy | Should -Match 'Close Unsloth or other llama\.cpp users' + } +} + +Describe 'the local-directory link path mirrors it, as its comment claims' { + It 'is present in setup.ps1' { + $script:LocalLinkBusy | Should -Not -BeNullOrEmpty + } + + It 'offers ACL repair on the install directory' { + $script:LocalLinkBusy | Should -Match 'repair the ACLs on \$LlamaCppDir' + } + + It 'still terminates with exit code 3' { + $script:LocalLinkBusy | Should -Match 'Exit-SetupFailure .+ 3' + } + + It 'still tells the user to close other llama.cpp users' { + $script:LocalLinkBusy | Should -Match 'Close Unsloth or other llama\.cpp users' + } +} + +Describe 'the whisper.cpp exit-3 branch remains the cause-neutral precedent' { + It 'is present in setup.ps1' { + $script:WhisperBusy | Should -Not -BeNullOrEmpty + } + + It 'names no process and no scanner' { + $script:WhisperBusy | Should -Not -Match '(?i)process|scanner' + } +} + +Describe 'a Node the installer kept because it could not be replaced' { + BeforeAll { + $script:NodeKept = $null + if ($script:NodeExit) { + $at = $script:NodeExit.IndexOf('keeping existing isolated Node') + if ($at -ge 0) { $script:NodeKept = $script:NodeExit.Substring($at) } + } + } + + It 'the Node exit handling is present in setup.ps1' { + $script:NodeExit | Should -Not -BeNullOrEmpty + } + + It 'matches the line the Node installer logs when it keeps an install' { + $script:NodeInstaller | Should -Match 'keeping existing isolated Node' + $script:NodeExit | Should -Match '\$nodeOut -match [''"]keeping existing isolated Node[''"]' + } + + It 'is reached only after both failure arms, so only on exit 0' { + $failure = $script:NodeExit.IndexOf('$nodeExit -ne 0') + $failure | Should -BeGreaterOrEqual 0 + $script:NodeExit.IndexOf('keeping existing isolated Node') | Should -BeGreaterThan $failure + } + + It 'warns and carries on rather than failing setup' { + $script:NodeKept | Should -Match 'step "node" "update not applied, existing isolated Node kept" "Yellow"' + $script:NodeKept | Should -Not -Match 'Exit-SetupFailure' + } + + It 'relays the installer output only when it carries the takeown/icacls lines' { + $script:NodeKept | Should -Match '(?s)\$nodeOut -match [''"]takeown /F[''"]\)\s*\{\s*Write-StudioLine \$nodeOut' + } +}