studio: report a denied rename as a possible ACL fault, not a scanner (#10075)

* studio: report a denied Node rename as a possible ACL fault, not a scanner

_replace_with_retry retries `os.replace` on WinError 5, 32 and 145 and logs one
message for all three: "a scanner is likely still holding the extracted files".
WinError 5 is not only a scanner. It is equally what a tree with unreadable ACLs
looks like, and that is a permission fault no backoff can clear. It matters most
because `_swap_into_place` also uses this helper to move the EXISTING install
aside, the tree a user is likeliest to have broken permissions on.

In #9928 the same rename in the llama.cpp installer failed with (5) on every
attempt and reported a scanner. The reporter's cause was corrupt ACLs -- icacls
and Get-Acl both returned access-denied for the directory -- and
`takeown /F ... /R /D Y` followed by `icacls ... /reset /T /C` fixed it. The Node
helper carries the same three codes and the same message.

5 stays in the retry set: a scanner does raise it right after extraction. Only
the reporting changes: the per-retry line offers both causes, and the
takeown/icacls recovery prints once, when the budget is exhausted rather than on
every attempt. WinError 32 and 145 keep their wording. The two commands go on
separate lines for the reason tests/studio/install/test_setup_denied_install_tree.py
pins -- on one line takeown swallows icacls as an argument. The constant and the
recovery lines live in this module because it imports nothing outside the stdlib
on purpose.

Three tests extend the suite that already covers the helper. Two fail without
the change: test_replace_names_acl_recovery_when_access_denied_persists and
test_replace_does_not_blame_a_scanner_for_access_denied.
test_replace_keeps_the_scanner_message_for_a_sharing_violation is a parity guard:
32 keeps its wording and prints no ACL guidance.

One consequence worth naming: a 5 that really was a scanner now prints ACL
guidance that does not apply to it, at the point the rename has already failed.

Refs #9928.

* studio: make ACL rename recovery actionable

* studio: address ACL recovery review feedback

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* studio: stop reporting every blocked llama.cpp install as an active process

setup.ps1 exits 3 when the existing llama.cpp install could not be replaced, and
both sites that report it asserted an active llama.cpp process as the cause.
Windows raises WinError 5 for a file another process holds open and for a tree
whose ACLs are unreadable alike; is_busy_lock_error in studio/prebuilt_core.py
classifies 5 as busy for exactly that reason, and _access_denied_recovery_lines
prints takeown/icacls guidance for the ACL case. Write-LlamaFailureLog relays
that guidance into the same block, so an ACL-denied user read repair commands
bracketed by a headline and a retry hint that both named a process instead.

Two sites, not one. The prebuilt exit-3 branch is the reported one; the
local-directory link path carries the same three lines and says in its own
comment that it mirrors the prebuilt handling, and that "Denied counts as
surviving: unreadable is not gone" -- so it reaches the message by the same
route and had to move with it.

The classification is unchanged: exit 3 still means blocked, is_busy_lock_error
still treats WinError 5 as busy, and the user is still told to close other
llama.cpp users. Only the claim about which cause it was is dropped, in favour
of naming both recoveries. whisper.cpp's exit-3 branch already reads "install
busy; keeping existing runtime" and is the cause-neutral precedent followed here.

Studio.Setup.BlockedInstallCause.Tests.ps1 pins it: 15 tests, 5 of which fail
without this change, including a file-wide guard that neither phrase reappears
at a third site and that every blocked-install site offers ACL repair.

One consequence worth naming: the local-link path names the directory but cannot
print the takeown/icacls lines, because it has no installer output to relay. It
points at the ACLs on that path and leaves the commands to Write-PathAccessDenied,
which already prints them wherever setup detects a denial up front.

* studio: gate ACL retry guidance on helper output

* studio: keep a running Node when its replacement stays denied

The re-raise of a reported access denial sat ahead of install_prebuilt's
keep-existing check, so a WinError 5 rename that outlasted the retries failed
setup even when the Node already on disk still ran: exit 1, which setup.ps1 and
setup.sh turn into "Could not install an isolated Node runtime". The re-raise
was there for a real reason -- both scripts show the installer's output only on
a non-zero exit, so a plain keep-existing return would have swallowed the
takeown/icacls lines -- but the two goals conflicted only because exit 0 was
silent.

It no longer is. install_llama_prebuilt.py already exits 0 when it keeps the
tree on disk after a transient failure, and setup.ps1 and setup.sh match its
"keeping the existing complete install" line to warn "update unavailable,
existing prebuilt kept". Node now takes the same shape: the reported denial
reaches the keep-existing check like any other failure, a Node that still runs
is kept with exit 0 on a line ending "keeping existing isolated Node", as the
download-failure case already logs, and both scripts match that line, warn
"update not applied, existing isolated Node kept", and relay the installer
output when it carries takeown lines. The line names the rename instead of
"Node download failed" when that is what failed. The warning is not llama.cpp's
text, which test_pr10648_offline_and_status.py holds to exactly two keep arms.

No new exit code, and nothing else moves: a Node that does not run, a forced
reinstall, and a same-version install whose digest is not the pin still raise,
exactly as they do for a failed download. setup.sh gets the arm for parity; its
takeown branch can only fire on Windows output.

test_install_prebuilt_keeps_existing_when_a_reported_denial_blocks_the_swap
replaces test_install_prebuilt_does_not_hide_reported_acl_denial and fails
without this change, when the swap's OSError escapes. The three cases of
test_install_prebuilt_reraises_a_reported_denial_it_cannot_keep_existing_through
are parity guards. Studio.Setup.BlockedInstallCause.Tests.ps1 gains a block on
the setup.ps1 arm, four of its five tests failing without it, and
tests/sh/test_setup_node_kept_existing.sh runs the real setup.sh block against a
stand-in installer, three of its thirteen checks failing without it.

One consequence worth naming: setup now carries on with a Node it could not
replace, so a user can stay on an older runtime while setup succeeds, and the
warning prints once per run.

Refs #9928.

* studio: offer the Node ACL repair only where the caller names the paths

_replace_with_retry printed takeown/icacls for its source whenever a WinError 5
outlasted the retries, and flagged the exception as a reported denial, whoever
called it. Since #10648 that includes atomic_replace_from_tempfile, which renames
a temp file onto the downloaded archive and onto the install marker. A denial
there named a temp file that is removed moments later, and the flag made the
keep-existing line say the existing Node "could not be replaced" when the swap
had never run.

The repair lines and the flag now need access_denied_paths, which only
_swap_into_place passes, for its three renames. A temp-file rename still retries
the same budget and still raises; it only stops naming a path that will not
exist, and a Node kept after it is logged as "Node download failed", as before.

test_a_denied_marker_write_offers_no_repair_for_its_temp_file drives
atomic_replace_from_tempfile into an exhausted 5 and fails without this change.
test_replace_names_acl_recovery_when_access_denied_persists now passes its paths,
as the callers that get the lines do.

Refs #9928.

* studio: describe the llama.cpp exit-3 repair gate by what it checks

The comment above setup.ps1's llama.cpp exit-3 branch said "The helper prints
takeown guidance only when access was denied", and the header of
Studio.Setup.BlockedInstallCause.Tests.ps1 named _access_denied_recovery_lines
as the source of llama.cpp's repair commands. Both described the installer
half this PR carried before the llama.cpp message was left to #10182.

On main no installer prints those lines: install_llama_prebuilt.py and
prebuilt_core.py contain no takeown at all, and _access_denied_recovery_lines
exists only in install_node_prebuilt.py. So the branch's `-match 'takeown /F'`
arm is correct but cannot fire until #10182 lands, and both comments said it
could.

The gate stays, because it is what #10182's output needs, and without it the
ACL repair hint would be offered to every busy install. Only the text changes:
both comments now describe the check rather than a producer.

Comments only, no assertion changes.

Refs #9928

* tests: anchor the Node exit-chain Pester block on the chain itself

Get-BlockSource took the Node exit handling from the first
`$nodeExit = $LASTEXITCODE` to the next closing brace at that indent. #11786
put the mirror retry between the two, `if ($nodeExit -notin 0, 3, 4 -and
(Use-MirrorSpare node)) { ... }`, so the slice ended at the retry's own brace.
Merged with main, four of the five tests on a kept Node failed against a
setup.ps1 that still has the branch they look for.

The marker is now `$nodeExit -eq 3`, the chain's first line, once in
setup.ps1. Against main's setup.ps1 the file still fails 11 of 22 (4, 3, 1, 3
and 0 by block), as it did at a09f3765b; with this branch's it passes 22.

* studio: print the Node ACL repair only when the existing Node is kept

Since #10533, a denied swap that cannot keep the existing Node reaches main()
as a PermissionError and exits 4, and setup.ps1 answers exit 4 with
Exit-PathAccessDenied: its own elevated takeown/icacls for the refused path
and a note that antivirus can deny it too. _replace_with_retry had already
printed ours, so that path showed two sets of repair commands for the same
folder, with different icacls flags, around #10533's advice that elevation
may not clear a security-software block.

The lines now ride on the error and install_prebuilt logs them only in the
branch that keeps a Node that still runs. That is exit 0, where setup.ps1
and setup.sh relay them on `takeown /F` and nothing else names a repair.
"rename still blocked (5) after N attempts" still prints at the retry, so the
exit-4 output keeps the rename's own record.

Measured through main() with the real swap and a refused rename: exit 4
printed our four repair commands ahead of #10533's message, and now prints
none, with the denied-path and denied-scope markers unchanged. A kept Node
prints the same lines as before, ahead of "existing Node could not be
replaced".

test_a_denial_that_cannot_keep_node_leaves_the_repair_to_setup fails
without this change; test_a_kept_node_prints_the_repair_setup_relays is a
parity guard for the kept path. Four tests that read the lines from the log
or checked the old flag, and the swap stand-in that set it, now use the error.

One consequence worth naming: when the rollback of a failed forward rename
is denied too, the repair lines naming the backup no longer print. The line
"could not restore the previous Node install from <backup>" still names it,
and the run exits 4.

* Trim comments in the denied-rename reporting

---------

Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <23090290+danielhanchen@users.noreply.github.com>
This commit is contained in:
Wayne Sletcher 2026-10-05 12:56:17 +02:00 • committed by GitHub
parent 3161553fc0
commit 2dc042ccfc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 607 additions and 17 deletions

View file

@ -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

View file

@ -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

View file

@ -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"

View file

@ -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"

View file

@ -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

View file

@ -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'
}
}