From ebfefcf84e1bd030396db64f14e67d4801a7d2b5 Mon Sep 17 00:00:00 2001 From: Michael Han <107991372+shimmyshimmer@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:25:39 -0700 Subject: [PATCH] Windows: do not abort setup on an unreadable llama.cpp install (#7735) * Windows: do not abort setup on an unreadable llama.cpp install Test-Path raises UnauthorizedAccessException instead of returning $false when an ACL denies the probe. setup.ps1 runs under $ErrorActionPreference = "Stop", so the bare probe of UNSLOTH_PREBUILT_INFO.json in the llama.cpp prebuilt phase killed setup with a raw "Test-Path : Access is denied" and exit code 1. The desktop app had nothing but [TAURI:ERROR_DEFAULT] to fall back on, so it showed "unsloth studio setup failed (exit code 1)". ~/.unsloth/llama.cpp sits beside the app, not inside it, so reinstalling reused the unreadable folder and hit the same line again, including a reinstall to a different drive. Add Get-PathState (Present / Absent / Denied) plus Test-PathQuiet, route the probes that read inside install trees we do not own through them, and report a denied llama.cpp install through Exit-SetupFailure so the reason and the recovery steps reach the desktop UI. Reported in unsloth-test/unsloth-test#9 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop on every denied path, and split the recovery commands Review follow-ups: - Assert-StudioOwnedOrAbsent treated a denied root as absent and returned, so the caller could go on to replace a tree it cannot read. Probe the root three-state and stop on Denied, still gated on $StudioHomeIsCustom so default-home behaviour is unchanged. - The source-build .git probe treated a denied checkout as "no checkout" and cloned a replacement. The swap that follows recursively removes the original and moves the temp tree over it under "Continue" and unchecked, so a denied child could leave a half-deleted install. Stop instead. This path already treated denied as absent before the previous commit (that probe runs under "Continue", so it printed an error and took the false branch), so the hazard is older than this branch, but it is in scope for the same reason. - Probe $LlamaCppDir itself three-state, so an unreadable parent is reported rather than dying on the bare probe under "Stop". - takeown and icacls were printed joined by "then", which is not a PowerShell separator: takeown would swallow the rest as arguments and icacls would never run. Print them on separate lines. Fold the repeated guidance into Exit-PathAccessDenied so all five denial routes report the same thing. * Harden the denial reporting path, found by simulation Ran the real decision blocks against simulated filesystems (denied file, denied parent, traverse-only and list-only dirs, symlinks, dangling links, wildcard and unicode paths, 3000 random paths) plus PSScriptAnalyzer's 5.1/6.2/7.0 syntax check. Two things came out of it: - Get-PathDenialDetail threw a parameter-binding exception on an empty path. It runs while a failure is being reported, so it would have replaced the actionable message with a raw binding error at exactly the wrong moment. Null and empty are now accepted and return no detail. - The link-target lookup used an empty catch, which PSScriptAnalyzer flags and which hid the intent. It assigns $null explicitly now. Both are covered by new checks. Also promoted the strongest invariant from the simulation into the suite: Get-PathState must agree with a bare Test-Path on every probe that did not throw, and Denied may only appear where the old probe threw, so no path that worked before can take a different branch now. Verified: PSUseCompatibleSyntax reports nothing for 5.1, 6.2 and 7.0; the Python contract tests pass on 3.10 through 3.13 in separate uv venvs; the tauri install:: unit tests pass (17), which is the code that prefers the [TAURI:ERROR] line over the generic exit-code message. * Trigger the Windows PowerShell tests when they change studio-windows-inference-smoke.yml runs six PowerShell unit tests out of tests/studio, but its pull_request paths filter matched none of them, and no other workflow runs them. A PR touching only one of those tests never ran it. Five predate this branch; the sixth is the ACL test added here. Scope the filter to tests/studio/*.ps1 rather than tests/studio/**, so a python-only change under that directory does not pull in the GGUF smoke jobs. This matches what the other two workflows already do: parity-ci lists its .ps1 test outright and update-smoke uses a scoped glob. Guard it in test_ci_shell_suite_coverage.py, which exists for this exact failure (tests/sh had the same hole): every tests/*.ps1 a workflow invokes must be matched by that workflow's paths filter, and must exist. The GitHub glob matcher it needs has its own table-driven test, since a wrong matcher would make the guard pass on everything. Verified by reverting the one-line filter change: the guard then names all six unrun tests. * Make the Windows PowerShell test step fail when a test fails Verifying the path-filter fix turned up a second hole in the same step. A `shell: pwsh` step inherits only the LAST command's exit code, and this step ran five tests as five bare commands, so only the last one could fail the build. test_resolve_cuda_toolkit.ps1 has been printing FAIL exits non-zero (scenario 2, forced source build) FAIL exits non-zero (scenario 6, no toolkit, forced) 2 check(s) FAILED on every Windows run, exiting 1, and the job reported success. Confirmed on main (run 30723608191, sha c67410a7), so it predates this branch, and it reproduces locally. The cause is in the test, not the installer. Resolve-CudaToolkit -RequireOrExit leaves through Exit-SetupFailure, which the child harness never stubbed, so under ErrorActionPreference=Continue the call was an ignored command-not-found, the child fell through and exited 0. The harness already injects the real Resolve-CudaToolkit and Write-CudaDriverToolkitMismatch by AST, so inject the real Exit-SetupFailure the same way. That test now passes 25/25. With it green, add the exit-code checks after each invocation, matching what studio-windows-update-smoke.yml already does, and guard the pattern in test_ci_shell_suite_coverage.py: any step running more than one PowerShell test must check $LASTEXITCODE after each. Verified by reverting each piece: dropping one check makes the guard name that test, and dropping the Exit-SetupFailure injection brings both scenario failures straight back. * Stop on a denied --with-llama-cpp-dir instead of reinstalling over it When UNSLOTH_LOCAL_LLAMA_CPP_DIR points at the canonical $LlamaCppDir and the llama-server.exe there is ACL-denied, Test-PathQuiet collapsed Denied to $false, $LocalLlamaServerFound stayed false, and the canonical branch reported "nothing built there yet; running the normal install". The prebuilt installer then moves that tree aside and replaces it, which is exactly what the branch's own comment says it exists to prevent. The old bare probe stopped first, by throwing under "Stop". Probe the candidates three-state and stop on Denied. Same for the directory probe itself, which reported an unreadable dir as "does not exist" and sent the user after the wrong problem. The generic message could not be reused as-is here: it tells the user to delete the folder because Unsloth reinstalls it, which is true of the managed cache and wrong for a build they pointed us at. Exit-PathAccessDenied takes -UserSupplied, which swaps that advice for restoring access or repointing UNSLOTH_LOCAL_LLAMA_CPP_DIR, and keeps the takeown/icacls lines. Verified by driving the real block through every state: a readable build is still reused, a genuinely empty canonical dir still falls through to the normal install, a missing dir still reports "does not exist", and a denied build now stops with exit 1 instead of being replaced. Reverting the probe puts the fall-through back, and the user-supplied path never prints "delete or rename" or "managed cache". * Carry the denial through three more probes Three review points, all reproduced before fixing: - The canonical --with-llama-cpp-dir override still got the managed advice ("delete it, Unsloth reinstalls it"). The override means the user asked to reuse whatever is in that tree, so deleting it is wrong wherever it sits. Both candidate denials now pass -UserSupplied, which collapses the branch to one call. - Phase 1b's git prerequisite scan probes the same candidate binaries with a bare Test-Path under "Stop", thousands of lines before the Phase 4 guards, so a denied override terminated the run with the raw error this change exists to replace. Reproduced, then guarded. - Test-StudioOwnedAdoptable collapsed a denied prebuilt marker to $false, so Assert-StudioOwnedOrAbsent called an Unsloth tree an unrelated directory and told the user to move it aside. Get-StudioAdoptableState returns Yes/No/Denied and the guard reports the denial first; Test-StudioOwnedAdoptable stays as the boolean view for the cosmetic cleanup gate. A denied file under a readable directory is a Windows-ACL-only state: POSIX keeps a mode-000 file stat-able, and a symlink into a denied directory still answers Test-Path. The local run injects that one state at the lowest seam and lets the real functions run; the Windows leg of test_path_probe_access_denied.ps1 builds it for real with icacls and skips elsewhere with the reason. test_setup_ps1_adopts_existing_whisper_prebuilt_marker sliced between two function names and the marker scan moved, so its anchor now points at Get-StudioAdoptableState. Its assertion is unchanged. Reverting each fix individually puts the original behaviour back: the ownership misdiagnosis, the raw "Access to the path ... is denied" from Phase 1b, and the delete-your-own-build advice. * Close the remaining denial gaps and the stale probe anchors for PR #7735 - tests/sh/test_with_llama_cpp_dir_flag.sh anchored the literal 'if ($ResolvedLocal -eq $LlamaCppDir) {', which 2a61343 hoisted into $LocalIsCanonical. Re-pin it to the comparison, not the branch. - The junction path deleted and replaced $LlamaCppDir behind a bare Test-Path, so a denied destination still threw raw under a default home. Probe it three-state, and treat Denied as surviving removal. - Get-Content on the prebuilt metadata still globbed while the probes gating it went literal, so a path holding [ or ] passed the probe and threw into the catch. Make both reads literal, with a test. - Get-PathDenialDetail could throw on a non-filesystem provider item whose .Attributes has no -band overload, replacing the failure being reported. - Win32Exception keeps E_FAIL in HResult and the code in NativeErrorCode, so the HRESULT check never matched it. Fix the comment and the check. - test_windows_git_gate.py ran the layout scan in a child that never defined Get-PathState, so de486ff had it silently report nothing built. Inject the real helpers, as test_resolve_cuda_toolkit.ps1 does. - Relax the exact Exit-PathAccessDenied count to a floor and key the -UserSupplied rule on the path reported rather than on position. * Stop advising deletion of a tree whose ownership cannot be read The ownership guard stops precisely because it could not read the marker, so it cannot claim the folder is ours either. It was still emitting the managed-cache text, telling the user to delete it. Eleven lines below, the readable-but-unowned branch says 'move it aside' instead, so we were being gentler when we could prove the tree was not ours than when we could not read it at all. New -OwnershipUnverified wording for those three stops. Also from re-reading the previous commit: - The reparse-point unlink above the junction path ran before the new three-state probe, and a link reports Present, so the probe could not cover it. A denied unlink still terminated on the raw .Delete() throw. - That junction destination probe had no test at all; reverting it left the suite green, since the count floor cannot see a swap. Pinned by name like every other route. - The re-pinned shell anchor accepted an assignment that nothing consumed. Pin the branch that uses it too. - Get-PathDenialDetail now checks the item type rather than the attribute type, which also covers a provider item with no Attributes at all. * Check both destructive steps of the temp-dir swap The guard above the clone path probes only .git, but its own comment names any unreadable child as the risk. A forced source build over a non-git install with a denied child elsewhere reads Absent, clones into a temp dir, and reaches the swap. Both steps there are non-terminating under Continue and neither was checked. Reproduced: Remove-Item partially fails, the original survives, and Move-Item then moves the temp dir INSIDE it, so the new binary lands at llama.cpp/llama.cpp.build./llama-server.exe while $LlamaServerBin points at llama.cpp/build/bin/Release. Setup carries on reporting success with no usable server and a half-deleted install. Check the removal before moving, so the stop happens while the temp build is still whole, and check the move afterwards. Denied routes through Exit-PathAccessDenied; anything else surviving exits 3 like the other blocked-replacement paths. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen --- .../studio-windows-inference-smoke.yml | 15 + studio/setup.ps1 | 268 ++++++++++++++++-- tests/python/test_windows_git_gate.py | 19 ++ tests/sh/test_with_llama_cpp_dir_flag.sh | 5 +- .../install/test_setup_denied_install_tree.py | 267 +++++++++++++++++ tests/studio/test_ci_shell_suite_coverage.py | 125 ++++++++ .../studio/test_path_probe_access_denied.ps1 | 243 ++++++++++++++++ tests/studio/test_resolve_cuda_toolkit.ps1 | 12 + tests/test_studio_install_workspace_guard.py | 10 +- 9 files changed, 937 insertions(+), 27 deletions(-) create mode 100644 tests/studio/install/test_setup_denied_install_tree.py create mode 100644 tests/studio/test_path_probe_access_denied.ps1 diff --git a/.github/workflows/studio-windows-inference-smoke.yml b/.github/workflows/studio-windows-inference-smoke.yml index d821664327..e263243e6b 100644 --- a/.github/workflows/studio-windows-inference-smoke.yml +++ b/.github/workflows/studio-windows-inference-smoke.yml @@ -27,6 +27,11 @@ on: - 'install.ps1' - 'pyproject.toml' - 'tests/studio_setup_ps1/**' + # The PowerShell unit tests this workflow runs live here. Without this a + # PR touching only one of them never exercises it, and no other workflow + # runs them. Scoped to *.ps1 so python-only changes under tests/studio do + # not pull in the GGUF smoke jobs. + - 'tests/studio/*.ps1' - '.github/workflows/studio-windows-inference-smoke.yml' push: branches: [main, pip] @@ -78,10 +83,20 @@ jobs: if ($errs) { $errs | ForEach-Object { $_.ToString() }; exit 1 } Write-Host "$f parsed with no errors" } + # Propagate each child test's failure before continuing: the step only + # inherits the LAST command's exit code, so without these every test + # but the last one could fail and still report green (studio-windows- + # update-smoke.yml already does this). pwsh -NoProfile -File tests/studio/test_resolve_cuda_toolkit.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_torch_flavor.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_node_decision.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_node_probe_guard.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } + pwsh -NoProfile -File tests/studio/test_path_probe_access_denied.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } # uninstall.ps1: native uninstall must keep the shared unsloth.ico while a # WSL shortcut still references it (dual install), else that shortcut blanks. diff --git a/studio/setup.ps1 b/studio/setup.ps1 index bf766198d1..37d67677ad 100644 --- a/studio/setup.ps1 +++ b/studio/setup.ps1 @@ -289,16 +289,127 @@ function Remove-AgentInstructionFiles { } } +# ERROR_ACCESS_DENIED in any of its disguises. PowerShell wraps .NET exceptions +# in MethodInvocationException, so walk the chain instead of catching by type. +function Test-AccessDeniedError { + param($ErrorRecord) + + $ex = if ($ErrorRecord -is [System.Management.Automation.ErrorRecord]) { $ErrorRecord.Exception } else { $ErrorRecord } + while ($ex) { + if ($ex -is [System.UnauthorizedAccessException]) { return $true } + # IOException carries ERROR_ACCESS_DENIED as an HRESULT; Win32Exception + # keeps E_FAIL there and puts the code in NativeErrorCode instead. + if ($ex.HResult -eq -2147024891) { return $true } + if ($ex -is [System.ComponentModel.Win32Exception] -and $ex.NativeErrorCode -eq 5) { return $true } + $ex = $ex.InnerException + } + if ($ErrorRecord -is [System.Management.Automation.ErrorRecord]) { + return ($ErrorRecord.CategoryInfo.Category -eq [System.Management.Automation.ErrorCategory]::PermissionDenied) + } + return $false +} + +# Test-Path throws UnauthorizedAccessException (it does not return $false) when +# an ACL denies the probe, and this script runs under "Stop", so a denied path +# aborted setup with a raw error. "Denied" is kept distinct from "Absent" +# because a denial needs reporting, not a silent retry. +function Get-PathState { + param( + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Path, + [ValidateSet("Any", "Leaf", "Container")][string]$PathType = "Any" + ) + + if ([string]::IsNullOrWhiteSpace($Path)) { return "Absent" } + try { + if (Test-Path -LiteralPath $Path -PathType $PathType -ErrorAction Stop) { return "Present" } + return "Absent" + } catch { + if (Test-AccessDeniedError $_) { return "Denied" } + # Malformed path, offline drive, dangling link: nothing usable there. + return "Absent" + } +} + +# Non-throwing Test-Path for paths inside install trees we do not control. +# Callers that must react to a denial use Get-PathState instead. +function Test-PathQuiet { + param( + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Path, + [ValidateSet("Any", "Leaf", "Container")][string]$PathType = "Any" + ) + + return ((Get-PathState -Path $Path -PathType $PathType) -eq "Present") +} + +# Names the link target of a denied dir, since that is where the user must look. +# Empty/null tolerated: this runs while reporting a failure and must not add one. +function Get-PathDenialDetail { + param([Parameter(Mandatory = $true)][AllowNull()][AllowEmptyString()][string]$Path) + + if ([string]::IsNullOrWhiteSpace($Path)) { return "" } + $item = Get-Item -LiteralPath $Path -Force -ErrorAction SilentlyContinue + if (-not $item) { return "" } + # Non-filesystem providers expose an unrelated .Attributes with no -band + # overload, and throwing here would replace the failure we are reporting. + if ($item -isnot [System.IO.FileSystemInfo]) { return "" } + if (-not ($item.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { return "" } + $target = $null + try { $target = $item.Target } catch { $target = $null } + # PS 5.1 exposes .Target as a collection; PS 7 as a string. + if ($target) { return " (it is a link to $(@($target) -join ', '))" } + return " (it is a link)" +} + +# One stop for every unreadable install tree. Nothing downstream (validate, +# replace, junction, source build, swap) can work without this access, and the +# folder outlives an app reinstall, so retrying cannot help. +function Exit-PathAccessDenied { + param( + [Parameter(Mandatory = $true)][AllowNull()][AllowEmptyString()][string]$Path, + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Label, + # "delete it, we reinstall it" is true of the managed cache and wrong for + # a tree the user pointed us at. Never tell them to delete their build. + [switch]$UserSupplied, + # Same rule, one step weaker: the ownership guard stops because it could + # not read the marker, so it cannot claim the tree is ours either. It + # already says "move it aside" when it CAN prove the tree is not ours. + [switch]$OwnershipUnverified + ) + + step "permissions" "$Label at $Path cannot be read: access is denied$(Get-PathDenialDetail -Path $Path)" "Red" + if ($UserSupplied) { + substep "Unsloth will not touch a directory you pointed it at, so this has to be fixed at the source" "Yellow" + substep "Restore access with these two in an elevated PowerShell, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build:" "Yellow" + } elseif ($OwnershipUnverified) { + substep "Unsloth cannot confirm this folder is its own install while it is unreadable, so it will not tell you to remove it" "Yellow" + substep "Restore access with these two in an elevated PowerShell, or move the folder aside and re-run setup:" "Yellow" + } else { + substep "This folder lives outside the app, so reinstalling Unsloth Studio, to any drive, reuses it and fails the same way" "Yellow" + substep "Simplest fix: close Unsloth, delete or rename $Path, then re-run setup (it is a managed cache and gets reinstalled)" "Yellow" + substep "If deleting is also denied, run these two in an elevated PowerShell, then re-run setup:" "Yellow" + } + substep "takeown /F `"$Path`" /R /D Y" "Yellow" + substep "icacls `"$Path`" /reset /T" "Yellow" + substep "Antivirus or Controlled folder access can deny this path too; allow or exclude it, then retry" "Yellow" + if ($UserSupplied) { + Exit-SetupFailure "Access denied reading $Label at $Path. Restore access with takeown/icacls, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build, then re-run setup." + } + if ($OwnershipUnverified) { + Exit-SetupFailure "Access denied reading $Label at $Path. Unsloth cannot confirm that folder is its own install while it is unreadable: restore access with takeown/icacls, or move it aside, then re-run setup." + } + Exit-SetupFailure "Access denied reading the existing $Label at $Path. Delete or rename that folder (Unsloth reinstalls it) or restore access with takeown/icacls, then re-run setup. Reinstalling the app does not reset it." +} + function Get-InstalledLlamaPrebuiltRelease { param([string]$InstallDir) $metadataPath = Join-Path $InstallDir "UNSLOTH_PREBUILT_INFO.json" - if (-not (Test-Path $metadataPath)) { + if (-not (Test-PathQuiet $metadataPath)) { return $null } try { - $payload = Get-Content $metadataPath -Raw | ConvertFrom-Json + $payload = Get-Content -LiteralPath $metadataPath -Raw | ConvertFrom-Json } catch { return $null } @@ -1764,7 +1875,13 @@ if (-not $HasGit) { if ($_localLlamaDir) { # Same layout candidates as the reuse check in Phase 4. foreach ($_c in @("llama-server.exe", "build\bin\llama-server.exe", "build\bin\Release\llama-server.exe")) { - if (Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)) { $_localLlamaBuilt = $true; break } + # Denied here terminated the run under "Stop" long before Phase 4's + # guarded probes, so this scan needs the same three-state handling. + $_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c) + if ($_cState -eq "Denied") { + Exit-PathAccessDenied -Path $_localLlamaDir -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied + } + if ($_cState -eq "Present") { $_localLlamaBuilt = $true; break } } } if (-not $_localLlamaBuilt) { @@ -2810,20 +2927,52 @@ $StudioHomeIsCustom = ($_studioHomeCanon -ne $LegacyStudioHome) # llama.cpp or whisper.cpp predating the .unsloth-studio-owned marker (see # setup.sh). Only Unsloth prebuilt markers count; source builds are # indistinguishable from a user clone on Windows and stay under the strict guard. +# "Yes" / "No" / "Denied". A denied marker is not evidence of absence: reading it +# as "No" makes the guard below call an Unsloth tree an unrelated directory and +# tell the user to move it aside, when the real problem is permissions. +function Get-StudioAdoptableState { + param([Parameter(Mandatory = $true)][string]$Path) + $denied = $false + foreach ($marker in @("UNSLOTH_PREBUILT_INFO.json", "UNSLOTH_WHISPER_PREBUILT_INFO.json")) { + switch (Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf) { + "Present" { return "Yes" } + "Denied" { $denied = $true } + } + } + if ($denied) { return "Denied" } + return "No" +} +# Boolean view for callers that only gate a cosmetic cleanup on adoption. function Test-StudioOwnedAdoptable { param([Parameter(Mandatory = $true)][string]$Path) - if (Test-Path -LiteralPath (Join-Path $Path "UNSLOTH_PREBUILT_INFO.json") -PathType Leaf) { return $true } - if (Test-Path -LiteralPath (Join-Path $Path "UNSLOTH_WHISPER_PREBUILT_INFO.json") -PathType Leaf) { return $true } - return $false + return ((Get-StudioAdoptableState -Path $Path) -eq "Yes") } function Assert-StudioOwnedOrAbsent { param( [Parameter(Mandatory = $true)][string]$Path, [Parameter(Mandatory = $true)][string]$Label ) - if (-not (Test-Path -LiteralPath $Path -PathType Container)) { return } - if ($StudioHomeIsCustom -and -not (Test-Path -LiteralPath (Join-Path $Path $StudioOwnedMarker) -PathType Leaf)) { - if (Test-StudioOwnedAdoptable $Path) { + # Denied is not Absent: a root we cannot read cannot be proven ours, and + # returning here would let the caller replace it. Both stops stay gated on + # $StudioHomeIsCustom, as before; a default-home denial is reported by the + # phase that owns the path. + $pathState = Get-PathState -Path $Path -PathType Container + if ($pathState -ne "Present") { + if ($StudioHomeIsCustom -and $pathState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + return + } + $markerState = Get-PathState -Path (Join-Path $Path $StudioOwnedMarker) -PathType Leaf + if ($StudioHomeIsCustom -and $markerState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + if ($StudioHomeIsCustom -and $markerState -ne "Present") { + $adoptState = Get-StudioAdoptableState -Path $Path + if ($adoptState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + if ($adoptState -eq "Yes") { Mark-StudioOwned $Path return } @@ -3847,7 +3996,13 @@ if ($LlamaPr) { $LocalLlamaCppLinked = $false $LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR if ($LocalLlamaCppSrc) { - if (-not (Test-Path -LiteralPath $LocalLlamaCppSrc -PathType Container)) { + # Unreadable is not missing: reporting "does not exist" would send the user + # looking for the wrong problem. + $localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container + if ($localSrcState -eq "Denied") { + Exit-PathAccessDenied -Path $LocalLlamaCppSrc -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR directory" -UserSupplied + } + if ($localSrcState -ne "Present") { step "llama.cpp" "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" "Red" Exit-SetupFailure "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" } @@ -3857,13 +4012,24 @@ if ($LocalLlamaCppSrc) { # layout LlamaCppBackend._layout_candidates() resolves (root-level, build\bin, # or build\bin\Release) so the flag never rejects a tree Unsloth could run. $LocalLlamaServerFound = $false + $LocalIsCanonical = ($ResolvedLocal -eq $LlamaCppDir) foreach ($_cand in @( (Join-Path $ResolvedLocal "llama-server.exe"), (Join-Path $ResolvedLocal "build\bin\llama-server.exe"), (Join-Path $ResolvedLocal "build\bin\Release\llama-server.exe"))) { - if (Test-Path -LiteralPath $_cand) { $LocalLlamaServerFound = $true; break } + # Denied must not read as "nothing built here": the canonical branch + # below would then hand the tree to the prebuilt installer, which + # replaces the very build this flag asked to reuse. + $candState = Get-PathState -Path $_cand + if ($candState -eq "Denied") { + # -UserSupplied even when this is the canonical location: the + # override says the tree is the user's build, so never advise + # deleting it, managed path or not. + Exit-PathAccessDenied -Path $ResolvedLocal -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied + } + if ($candState -eq "Present") { $LocalLlamaServerFound = $true; break } } - if ($ResolvedLocal -eq $LlamaCppDir) { + if ($LocalIsCanonical) { # Points at the canonical install location itself: never delete-then-link # onto itself. Reuse an existing build here (skip prebuilt + source) so the # staged prebuilt installer can't replace a build the user asked to reuse; @@ -3893,17 +4059,31 @@ if ($LocalLlamaCppSrc) { # can remove it and relink to a new valid directory. $existing = Get-Item -LiteralPath $LlamaCppDir -Force -ErrorAction SilentlyContinue if ($existing -and ($existing.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { - $existing.Delete() + # A link reads Present, so the probe below cannot cover a denied + # unlink; report it here rather than terminate on the raw throw. + try { $existing.Delete() } + catch { + if (Test-AccessDeniedError $_) { Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" } + throw + } } if ($StudioHomeIsCustom) { Assert-StudioOwnedOrAbsent -Path $LlamaCppDir -Label "llama.cpp install" } - if (Test-Path -LiteralPath $LlamaCppDir) { + # The destination is about to be deleted and replaced, so a denial here + # must stop rather than throw raw: under a default home nothing above + # has probed it three-state. + $destState = Get-PathState -Path $LlamaCppDir + if ($destState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($destState -eq "Present") { 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. - if (Test-Path -LiteralPath $LlamaCppDir) { + # Denied counts as surviving: unreadable is not gone. + 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 @@ -3938,15 +4118,27 @@ if ($LocalLlamaCppLinked) { substep "Skipping prebuilt install -- falling back to source build" "Yellow" } else { Write-Host "" - if (Test-Path -LiteralPath $LlamaCppDir) { + # Denied on the dir itself means an unreadable parent; Denied on the file + # below means an unreadable install. Either way nothing here can proceed, + # and the bare probes used to die under "Stop" with a raw + # "Test-Path : Access is denied" and exit 1. + $llamaDirState = Get-PathState -Path $LlamaCppDir + if ($llamaDirState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($llamaDirState -eq "Present") { substep "Existing llama.cpp install detected -- validating staged prebuilt update before replacement" # If the existing install is the wrong kind (e.g. windows-cpu on a ROCm # machine that should have windows-rocm), remove it so the installer is # forced to download the correct variant rather than skipping on tag match. $existingMetaPath = Join-Path $LlamaCppDir "UNSLOTH_PREBUILT_INFO.json" - if (Test-Path $existingMetaPath) { + $existingMetaState = Get-PathState -Path $existingMetaPath -PathType Leaf + if ($existingMetaState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($existingMetaState -eq "Present") { try { - $existingMeta = Get-Content $existingMetaPath -Raw | ConvertFrom-Json + $existingMeta = Get-Content -LiteralPath $existingMetaPath -Raw | ConvertFrom-Json $existingKind = $existingMeta.install_kind # A ROCm host may legitimately carry the fork's windows-rocm bundle # or the upstream windows-hip fallback, so accept either and never @@ -4092,7 +4284,7 @@ if ($LocalLlamaCppLinked) { (Join-Path $LlamaCppDir "llama-server.exe"), (Join-Path $LlamaCppDir "build\bin\llama-server.exe"), (Join-Path $LlamaCppDir "build\bin\Release\llama-server.exe"))) { - if (Test-Path -LiteralPath $_cand) { $PreservedLlamaServerFound = $true; break } + if (Test-PathQuiet $_cand) { $PreservedLlamaServerFound = $true; break } } if (-not $PreservedLlamaServerFound) { $script:LlamaCppDegraded = $true } # A preserved CUDA/ROCm/CPU server does not satisfy an explicit Vulkan @@ -4192,7 +4384,7 @@ if ($env:WHISPER_SERVER_PATH -or $env:UNSLOTH_WHISPER_CPP_PATH) { } $installedWhisperLlamaTag = "unknown" $llamaMarker = Join-Path $LlamaCppDir "UNSLOTH_PREBUILT_INFO.json" - if (Test-Path -LiteralPath $llamaMarker -PathType Leaf) { + if (Test-PathQuiet $llamaMarker "Leaf") { try { $markerPayload = Get-Content -LiteralPath $llamaMarker -Raw | ConvertFrom-Json if ($markerPayload.release_tag) { $installedWhisperLlamaTag = $markerPayload.release_tag } @@ -4501,7 +4693,15 @@ if ($LocalLlamaCppLinked) { $UseConcreteRef = ($ResolvedSourceRef -ne "latest" -and -not [string]::IsNullOrWhiteSpace($ResolvedSourceRef)) - if (Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")) { + # Denied must not read as "no checkout here": the fresh-clone branch ends in + # a swap that recursively removes this tree and moves the temp one over it, + # under "Continue" and unchecked, so an unreadable child would leave a + # half-deleted install behind. Stop while that is still avoidable. + $llamaGitState = Get-PathState -Path (Join-Path $LlamaCppDir ".git") + if ($llamaGitState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($llamaGitState -eq "Present") { # why: in-place git mutation (remote set-url, checkout -B, clean -fdx) # rewrites $LlamaCppDir; mirror the prebuilt and temp-dir-swap guards # so an unrelated workspace .git tree is never silently overwritten. @@ -4818,8 +5018,30 @@ if ($LocalLlamaCppLinked) { # Swap temp build dir into final location (only if we built in a temp dir) if ($BuildOk -and $LlamaCppDir -ne $OriginalLlamaCppDir) { Assert-StudioOwnedOrAbsent -Path $OriginalLlamaCppDir -Label "llama.cpp install" - if (Test-Path -LiteralPath $OriginalLlamaCppDir) { Remove-Item -LiteralPath $OriginalLlamaCppDir -Recurse -Force } + if ((Get-PathState -Path $OriginalLlamaCppDir) -ne "Absent") { + Remove-Item -LiteralPath $OriginalLlamaCppDir -Recurse -Force -ErrorAction SilentlyContinue + # Any unreadable or locked child survives the removal, and Move-Item + # then nests the build *inside* the leftovers instead of replacing + # them. Both are non-terminating here, so check before destroying + # more: the temp build is still whole at this point. + $swapState = Get-PathState -Path $OriginalLlamaCppDir + if ($swapState -eq "Denied") { + Exit-PathAccessDenied -Path $OriginalLlamaCppDir -Label "llama.cpp install" + } + if ($swapState -ne "Absent") { + step "llama.cpp" "could not replace the existing install at $OriginalLlamaCppDir" "Red" + substep "Part of it survived removal; the new build is intact at $LlamaCppDir" "Yellow" + substep "Close Unsloth and other llama.cpp users, or move that folder aside, then re-run setup" "Yellow" + Exit-SetupFailure "llama.cpp install at $OriginalLlamaCppDir could not be replaced; the new build is at $LlamaCppDir" 3 + } + } Move-Item -LiteralPath $LlamaCppDir -Destination $OriginalLlamaCppDir + # A failed move is non-terminating too; without this setup would report + # a build it never installed. + if ((Get-PathState -Path $LlamaCppDir) -ne "Absent") { + step "llama.cpp" "could not move the new build into $OriginalLlamaCppDir" "Red" + Exit-SetupFailure "llama.cpp build at $LlamaCppDir could not be moved into $OriginalLlamaCppDir" 3 + } $LlamaCppDir = $OriginalLlamaCppDir $BuildDir = Join-Path $LlamaCppDir "build" $LlamaServerBin = Join-Path $BuildDir "bin\Release\llama-server.exe" @@ -4865,7 +5087,7 @@ $llamaCppItem = Get-Item -LiteralPath $LlamaCppDir -Force -ErrorAction SilentlyC $llamaCppIsLink = $llamaCppItem -and ($llamaCppItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) if (-not $llamaCppIsLink -and ( -not $StudioHomeIsCustom -or - (Test-Path -LiteralPath (Join-Path $LlamaCppDir $StudioOwnedMarker) -PathType Leaf) -or + (Test-PathQuiet (Join-Path $LlamaCppDir $StudioOwnedMarker) "Leaf") -or (Test-StudioOwnedAdoptable $LlamaCppDir) )) { Remove-AgentInstructionFiles -Roots @($LlamaCppDir) diff --git a/tests/python/test_windows_git_gate.py b/tests/python/test_windows_git_gate.py index 60de430191..678a95c7bf 100644 --- a/tests/python/test_windows_git_gate.py +++ b/tests/python/test_windows_git_gate.py @@ -36,11 +36,30 @@ def _git_gate_block() -> str: raise AssertionError("Unclosed git gate block in setup.ps1") +def _function(name: str) -> str: + """Inject the real helper the block calls; an undefined one is a silent no-op + under Continue, which would let the layout scan always report 'nothing built'.""" + source = SETUP_PS1.read_text(encoding = "utf-8") + start = source.index(f"function {name} {{") + depth = 0 + for index in range(source.index("{", start), len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return source[start : index + 1] + raise AssertionError(f"Unclosed function {name} in setup.ps1") + + def _script() -> str: return f""" $DefaultLlamaPrForce = "0" $DefaultLlamaSource = "https://github.com/ggml-org/llama.cpp" $DefaultLlamaTag = "latest" +{_function("Test-AccessDeniedError")} +{_function("Get-PathState")} +function Exit-PathAccessDenied {{ param($Path, $Label, [switch]$UserSupplied) throw "denied: $Path" }} {_git_gate_block()} Write-Output $gitNeeded """ diff --git a/tests/sh/test_with_llama_cpp_dir_flag.sh b/tests/sh/test_with_llama_cpp_dir_flag.sh index cee158bf40..cb8b817d73 100644 --- a/tests/sh/test_with_llama_cpp_dir_flag.sh +++ b/tests/sh/test_with_llama_cpp_dir_flag.sh @@ -159,7 +159,10 @@ assert_contains \ "$SETUP_SH" 'if [ "$_RESOLVED_LOCAL" = "$_CANON_LLAMA_CPP_DIR" ]; then' assert_contains \ "setup.ps1: ignores a local dir equal to the canonical install location" \ - "$SETUP_PS1" 'if ($ResolvedLocal -eq $LlamaCppDir) {' + "$SETUP_PS1" '$LocalIsCanonical = ($ResolvedLocal -eq $LlamaCppDir)' +assert_contains \ + "setup.ps1: the canonical check actually gates the no-op branch" \ + "$SETUP_PS1" 'if ($LocalIsCanonical) {' echo "" echo "=== Results ===" diff --git a/tests/studio/install/test_setup_denied_install_tree.py b/tests/studio/install/test_setup_denied_install_tree.py new file mode 100644 index 0000000000..648b221824 --- /dev/null +++ b/tests/studio/install/test_setup_denied_install_tree.py @@ -0,0 +1,267 @@ +# SPDX-License-Identifier: AGPL-3.0-only +"""Contract checks for setup.ps1 on an unreadable llama.cpp install tree. + +Windows Test-Path raises UnauthorizedAccessException instead of returning +$false when an ACL denies the probe. setup.ps1 runs under "Stop", so the bare +probe of the prebuilt metadata aborted setup with a raw "Test-Path : Access is +denied" and exit code 1, which the desktop app showed as "unsloth studio setup +failed (exit code 1)". + +~/.unsloth/llama.cpp lives beside the app rather than inside it, so a reinstall, +even to another drive, reused the unreadable folder and failed on the same line. +The probes now go through Get-PathState / Test-PathQuiet and a denial produces +an actionable [TAURI:ERROR] message. + +Behavioural coverage against a real ACL-denied directory lives in +tests/studio/test_path_probe_access_denied.ps1. +""" + +import re +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[3] +SETUP_PS1 = (ROOT / "studio" / "setup.ps1").read_text(encoding = "utf-8") + + +def test_setup_defines_non_throwing_path_probes(): + for name in ("Test-AccessDeniedError", "Get-PathState", "Test-PathQuiet"): + assert re.search(rf"^function {re.escape(name)} \{{", SETUP_PS1, re.M), name + # Get-PathState must keep the three-way answer: collapsing "Denied" into + # "Absent" would hide the failure again instead of reporting it. + for state in ('return "Present"', 'return "Absent"', 'return "Denied"'): + assert state in SETUP_PS1 + + +def test_prebuilt_metadata_probe_cannot_terminate_setup(): + assert "if (Test-Path $existingMetaPath)" not in SETUP_PS1 + assert "$existingMetaState = Get-PathState -Path $existingMetaPath -PathType Leaf" in SETUP_PS1 + assert '$existingMetaState -eq "Denied"' in SETUP_PS1 + assert '$existingMetaState -eq "Present"' in SETUP_PS1 + + +def test_every_denial_route_reports_instead_of_proceeding(): + """An unreadable parent dir, metadata file, .git checkout, or ownership root + must all stop. Treating any of them as absent lets the caller replace or + delete a tree it cannot read.""" + assert "$llamaDirState = Get-PathState -Path $LlamaCppDir" in SETUP_PS1 + assert '$llamaDirState -eq "Denied"' in SETUP_PS1 + assert '$llamaGitState = Get-PathState -Path (Join-Path $LlamaCppDir ".git")' in SETUP_PS1 + assert '$llamaGitState -eq "Denied"' in SETUP_PS1 + assert "$pathState = Get-PathState -Path $Path -PathType Container" in SETUP_PS1 + assert '$StudioHomeIsCustom -and $pathState -eq "Denied"' in SETUP_PS1 + # The junction path replaces this destination, so it needs its own stop. + assert "$destState = Get-PathState -Path $LlamaCppDir" in SETUP_PS1 + assert '$destState -eq "Denied"' in SETUP_PS1 + # Denied counts as surviving removal; collapsing it would junction over it. + assert '(Get-PathState -Path $LlamaCppDir) -ne "Absent"' in SETUP_PS1 + # Floor, not an exact count: losing a route is the bug, adding one is not. + # Each route above is pinned by name, so a swap cannot hide under the floor. + assert SETUP_PS1.count("Exit-PathAccessDenied -Path") >= 9 + + +def test_denied_install_reports_an_actionable_failure(): + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "cannot be read: access is denied" in body + # The reporter reinstalled to a different drive and hit the same line; the + # message has to say why that cannot help. + assert "reinstalling Unsloth Studio, to any drive, reuses it" in body + assert "delete or rename $Path" in body + assert "Controlled folder access" in body + assert 'Exit-SetupFailure "Access denied reading the existing $Label' in body + assert "Reinstalling the app does not reset it." in body + + +def test_recovery_commands_are_separately_runnable(): + """On one line "then" is not a PowerShell separator: takeown would take the + rest as arguments and icacls would never run.""" + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + command_lines = [ + line for line in body.splitlines() if "takeown /F" in line or "/reset /T" in line + ] + assert len(command_lines) == 2, command_lines + assert not any("takeown" in line and "icacls" in line for line in command_lines), command_lines + + +def test_failure_reaches_the_desktop_ui(): + """Exit-SetupFailure is the only path that emits [TAURI:ERROR], which the + desktop app prefers over its generic exit-code message.""" + body = SETUP_PS1.split("function Exit-SetupFailure", 1)[1].split("\n}", 1)[0] + assert "UNSLOTH_TAURI_MODE" in body + assert "[TAURI:ERROR] $singleLine" in body + + +def test_ownership_guard_distinguishes_denied_from_unowned(): + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + assert ( + "$markerState = Get-PathState -Path (Join-Path $Path $StudioOwnedMarker) -PathType Leaf" + in guard + ) + assert '$markerState -eq "Denied"' in guard + # The old wording blamed ownership, which is unknowable while the tree is + # unreadable; it must stay for the genuinely-unowned case only. + assert "is not marked as an Unsloth-owned $Label" in guard + # Both stops stay gated, so default-home installs behave exactly as before. + assert guard.count("$StudioHomeIsCustom -and") == 3 + + +def test_no_bare_test_path_probes_inside_the_llama_install_tree(): + """Probes that read *inside* a tree whose permissions we do not control are + the ones that throw; they must all go through the guarded helpers.""" + inside_tree = re.compile( + r"Test-Path\b[^\n]*(\$existingMetaPath|\$llamaMarker|\$_cand|Join-Path \$LlamaCppDir)" + ) + offenders = [ + f"{index}: {line.strip()}" + for index, line in enumerate(SETUP_PS1.splitlines(), start = 1) + if inside_tree.search(line) + ] + assert not offenders, offenders + + +def test_metadata_reads_are_literal_like_the_probes_that_gate_them(): + """A literal probe followed by a globbing read still fails on a path holding + [ or ], so the probe passes and the read throws into the catch.""" + offenders = [ + f"{index}: {line.strip()}" + for index, line in enumerate(SETUP_PS1.splitlines(), start = 1) + if re.search(r"Get-Content\b[^\n]*(\$metadataPath|\$existingMetaPath|\$llamaMarker)", line) + and "-LiteralPath" not in line + ] + assert not offenders, offenders + + +def test_whisper_phase_stays_non_fatal_on_a_denied_llama_tree(): + """whisper.cpp failures degrade to Transformers dictation by contract, so + its read of the llama.cpp marker must not be able to terminate setup.""" + assert 'if (Test-PathQuiet $llamaMarker "Leaf")' in SETUP_PS1 + + +def test_reporting_helpers_tolerate_an_empty_path(): + """These run while a failure is being reported. A mandatory [string] rejects + null and empty, so without these attributes the reporter would throw a + binding exception instead of printing the actionable message.""" + for name in ("Get-PathDenialDetail", "Exit-PathAccessDenied"): + body = SETUP_PS1.split(f"function {name}", 1)[1].split("\nfunction ", 1)[0] + head = body.split("$Path", 1)[0] + assert "[AllowNull()]" in head, name + assert "[AllowEmptyString()]" in head, name + detail = SETUP_PS1.split("function Get-PathDenialDetail", 1)[1].split("\nfunction ", 1)[0] + assert 'if ([string]::IsNullOrWhiteSpace($Path)) { return "" }' in detail + + +def test_local_llama_dir_probes_are_three_state(): + """--with-llama-cpp-dir pointed at the canonical location reuses whatever is + built there so the prebuilt installer cannot replace it. A denied binary read + as "nothing built" put that replacement back, which is what the branch exists + to prevent.""" + assert "$localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container" in SETUP_PS1 + assert '$localSrcState -eq "Denied"' in SETUP_PS1 + local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1] + local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0] + assert "$candState = Get-PathState -Path $_cand" in local_block + assert '$candState -eq "Denied"' in local_block + assert '$candState -eq "Present"' in local_block + assert "Test-PathQuiet $_cand" not in local_block + # The disk-space branch keeps Test-PathQuiet on purpose: it only decides + # whether a preserved binary is usable, and an unreadable one is not. + + +def test_phase_1b_git_scan_is_guarded_too(): + """The git prerequisite scan probes the same candidate binaries in Phase 1b, + thousands of lines before the Phase 4 guards, and under "Stop". A denial + there reproduced the original raw termination.""" + scan = SETUP_PS1.split("$_localLlamaBuilt = $false", 1)[1].split( + "if (-not $_localLlamaBuilt) {", 1 + )[0] + assert "$_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c)" in scan + assert '$_cState -eq "Denied"' in scan + assert '$_cState -eq "Present"' in scan + assert "Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)" not in scan + assert "-UserSupplied" in scan + + +def test_adoption_markers_keep_their_denial(): + """A denied prebuilt marker is not evidence of absence. Collapsing it made the + ownership guard call an Unsloth tree an unrelated directory and tell the user + to move it aside, hiding a permissions problem.""" + assert "function Get-StudioAdoptableState" in SETUP_PS1 + state = SETUP_PS1.split("function Get-StudioAdoptableState", 1)[1].split("\nfunction ", 1)[0] + assert "Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf" in state + for verdict in ('return "Yes"', 'return "No"', 'return "Denied"'): + assert verdict in state + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + assert "$adoptState = Get-StudioAdoptableState -Path $Path" in guard + assert '$adoptState -eq "Denied"' in guard + # The denial must be reported before the "not Unsloth-owned" wording. + assert guard.index('$adoptState -eq "Denied"') < guard.index( + "is not marked as an Unsloth-owned" + ) + + +def test_user_supplied_paths_are_never_told_to_delete_themselves(): + """The managed advice ("delete it, Unsloth reinstalls it") is wrong for a tree + the user pointed us at with UNSLOTH_LOCAL_LLAMA_CPP_DIR.""" + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "[switch]$UserSupplied" in body + user_branch = body.split("if ($UserSupplied) {", 1)[1].split("} else {", 1)[0] + assert "delete or rename" not in user_branch + assert "managed cache" not in user_branch + assert "UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build" in user_branch + # Every call site that reports a path the user pointed us at must pass the + # switch, including the canonical location: the override says that tree is + # the user's build, so "delete it, we reinstall it" is wrong there too. + for line in SETUP_PS1.splitlines(): + if "Exit-PathAccessDenied" in line and "UNSLOTH_LOCAL_LLAMA_CPP_DIR" in line: + assert "-UserSupplied" in line, line + # Keyed on the path being reported, not on position: this block also reports + # the managed destination ($LlamaCppDir), where "delete it" is the right advice. + local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1] + local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0] + for line in local_block.splitlines(): + if "Exit-PathAccessDenied" not in line: + continue + if "$ResolvedLocal" in line or "$LocalLlamaCppSrc" in line: + assert "-UserSupplied" in line, line + elif "$LlamaCppDir" in line: + assert "-UserSupplied" not in line, line + + +def test_the_ownership_guard_never_advises_deleting_an_unverified_tree(): + """The guard stops because it could not read the marker, so it cannot claim + the tree is ours. It already says "move it aside" when it can prove it.""" + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + calls = [line.strip() for line in guard.splitlines() if "Exit-PathAccessDenied" in line] + assert len(calls) == 3, calls + for line in calls: + assert "-OwnershipUnverified" in line, line + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "[switch]$OwnershipUnverified" in body + branch = body.split("} elseif ($OwnershipUnverified) {", 1)[1].split("} else {", 1)[0] + assert "delete" not in branch.lower(), branch + assert "managed cache" not in branch, branch + assert "move the folder aside" in branch, branch + + +def test_the_reparse_point_unlink_reports_a_denied_delete(): + """A link probes Present, so the destination check below cannot cover it.""" + block = SETUP_PS1.split("$existing = Get-Item -LiteralPath $LlamaCppDir", 1)[1] + block = block.split("$destState", 1)[0] + assert "try { $existing.Delete() }" in block, block + assert "Test-AccessDeniedError" in block, block + + +def test_the_temp_dir_swap_checks_both_of_its_destructive_steps(): + """Remove-Item and Move-Item are both non-terminating here, and Move-Item + onto a surviving directory nests the build inside it rather than replacing + it, so setup would report a build it never installed.""" + swap = SETUP_PS1.split("# Swap temp build dir into final location", 1)[1] + swap = swap.split("} elseif (-not $BuildOk", 1)[0] + assert "$swapState = Get-PathState -Path $OriginalLlamaCppDir" in swap, swap + assert '$swapState -eq "Denied"' in swap, swap + # Stop before the move runs, while the temp build is still whole. + move = "Move-Item -LiteralPath $LlamaCppDir" + assert swap.index('$swapState -ne "Absent"') < swap.index(move), swap + # And catch a move that silently did not happen. + assert '(Get-PathState -Path $LlamaCppDir) -ne "Absent"' in swap.split(move, 1)[1], swap + assert "Test-Path -LiteralPath $OriginalLlamaCppDir" not in swap, swap diff --git a/tests/studio/test_ci_shell_suite_coverage.py b/tests/studio/test_ci_shell_suite_coverage.py index 50ed22f9de..4b7768a9b6 100644 --- a/tests/studio/test_ci_shell_suite_coverage.py +++ b/tests/studio/test_ci_shell_suite_coverage.py @@ -195,3 +195,128 @@ class TestBackendCiPathFilters: if __name__ == "__main__": pytest.main([__file__, "-v"]) + + +def _github_path_matcher(pattern: str) -> re.Pattern: + """GitHub path filters: ** crosses directories, * and ? do not.""" + out, i = [], 0 + while i < len(pattern): + c = pattern[i] + if pattern.startswith("**", i): + out.append(".*") + i += 2 + elif c == "*": + out.append("[^/]*") + i += 1 + elif c == "?": + out.append("[^/]") + i += 1 + else: + out.append(re.escape(c)) + i += 1 + return re.compile("^" + "".join(out) + "$") + + +def _workflows_running_powershell_tests(): + """Every workflow that invokes a tests/**.ps1 file, with its PR path filter.""" + found = {} + for workflow in sorted(_WORKFLOWS.glob("*.yml")): + text = workflow.read_text(encoding = "utf-8") + invoked = sorted(set(re.findall(r"pwsh -NoProfile -File (tests/[^\s`\"']+\.ps1)", text))) + if not invoked: + continue + parsed = yaml.safe_load(text) + # PyYAML parses the `on:` key as the boolean True. + triggers = parsed.get(True, parsed.get("on", {})) or {} + paths = (triggers.get("pull_request") or {}).get("paths") + found[workflow.name] = (invoked, paths) + return found + + +class TestGithubPathMatcher: + """The guard below is only as good as this matcher; a wrong one would pass + everything silently.""" + + @pytest.mark.parametrize( + "pattern,path,expected", + [ + ("tests/studio/*.ps1", "tests/studio/test_x.ps1", True), + ("tests/studio/*.ps1", "tests/studio/nested/test_x.ps1", False), + ("tests/studio/*.ps1", "tests/studio/test_x.py", False), + ("tests/studio/**", "tests/studio/nested/test_x.ps1", True), + ("studio/**", "studio/setup.ps1", True), + ("studio/**", "tests/studio/setup.ps1", False), + ( + "tests/studio/test_uninstall_*.ps1", + "tests/studio/test_uninstall_arg_guard.ps1", + True, + ), + ("tests/studio/test_uninstall_*.ps1", "tests/studio/test_node_decision.ps1", False), + ("install.ps1", "install.ps1", True), + ("install.ps1", "studio/install.ps1", False), + ], + ) + def test_matcher_semantics(self, pattern, path, expected): + assert bool(_github_path_matcher(pattern).match(path)) is expected + + +class TestPowerShellTestsRunOnAPr: + """tests/sh had this exact hole (see the module docstring) and so did the + Windows side: studio-windows-inference-smoke.yml ran six PowerShell tests + while its path filter matched none of them, so a PR fixing one of those + tests never ran it.""" + + def test_some_workflow_runs_powershell_tests(self): + assert ( + _workflows_running_powershell_tests() + ), "no workflow invokes a tests/*.ps1 file; did the invocation form change?" + + def test_every_invoked_powershell_test_triggers_its_workflow(self): + unguarded = [] + for name, (invoked, paths) in _workflows_running_powershell_tests().items(): + if paths is None: + continue # no filter at all means it always runs + matchers = [_github_path_matcher(p) for p in paths] + for test in invoked: + if not any(m.match(test) for m in matchers): + unguarded.append(f"{name} runs {test} but its paths filter never matches it") + assert not unguarded, ( + "these PowerShell tests can break without any PR running them; add the " + f"path (or a scoped glob) to the workflow's paths filter: {unguarded}" + ) + + def test_multi_test_steps_propagate_each_exit_code(self): + """A `shell: pwsh` step inherits only the LAST command's exit code, so a + step running several tests must check $LASTEXITCODE after each one. + Without it, test_resolve_cuda_toolkit.ps1 failed two checks on every + Windows run for as long as anyone can tell, and CI stayed green.""" + offenders = [] + for workflow in sorted(_WORKFLOWS.glob("*.yml")): + for block in re.findall( + r"run: \|\n(.*?)(?=\n [-a-zA-Z]|\Z)", + workflow.read_text(encoding = "utf-8"), + re.S, + ): + invocations = re.findall( + r"pwsh -NoProfile -File (tests/[^\s`\"']+\.ps1)[^\n]*\n(.*?)(?=pwsh -NoProfile -File|\Z)", + block, + re.S, + ) + if len(invocations) < 2: + continue # a single invocation's exit code is the step's + for test, following in invocations: + if "$LASTEXITCODE" not in following: + offenders.append(f"{workflow.name}: {test} runs without an exit-code check") + assert not offenders, ( + "these tests can fail without failing their step; add " + f"`if ($LASTEXITCODE) {{ exit $LASTEXITCODE }}` after each: {offenders}" + ) + + def test_every_invoked_powershell_test_exists(self): + missing = [ + f"{name} -> {test}" + for name, (invoked, _) in _workflows_running_powershell_tests().items() + for test in invoked + if not (REPO_ROOT / test).is_file() + ] + assert not missing, f"workflows invoke PowerShell tests that do not exist: {missing}" diff --git a/tests/studio/test_path_probe_access_denied.ps1 b/tests/studio/test_path_probe_access_denied.ps1 new file mode 100644 index 0000000000..96aa1d64a4 --- /dev/null +++ b/tests/studio/test_path_probe_access_denied.ps1 @@ -0,0 +1,243 @@ +# Regression test for setup.ps1 path probes on an ACL-denied install tree. +# +# Test-Path raises UnauthorizedAccessException instead of returning $false when +# an ACL denies the probe, and setup.ps1 runs under "Stop", so the bare probe of +# the llama.cpp prebuilt metadata aborted setup with a raw "Test-Path : Access +# is denied" and exit code 1. ~/.unsloth/llama.cpp outlives an app reinstall, so +# reinstalling, even to another drive, hit the same line again. +# +# The probes now go through Get-PathState / Test-PathQuiet, which never +# terminate and keep "Denied" distinct from "Absent". This runs the real +# functions against a genuinely unreadable directory (chmod on Unix, icacls deny +# on Windows). +$ErrorActionPreference = "Stop" +$script:failures = 0 +function Check($name, $cond) { + if ($cond) { Write-Host " PASS $name" } + else { Write-Host " FAIL $name" -ForegroundColor Red; $script:failures++ } +} + +$repoRoot = (Resolve-Path ([System.IO.Path]::Combine($PSScriptRoot, "..", ".."))).Path +$setupPath = [System.IO.Path]::Combine($repoRoot, "studio", "setup.ps1") +. ([System.IO.Path]::Combine($repoRoot, "tests", "studio_setup_ps1", "Get-FunctionSource.ps1")) + +foreach ($fn in @("Test-AccessDeniedError", "Get-PathState", "Test-PathQuiet", + "Get-PathDenialDetail", "Get-StudioAdoptableState", + "Test-StudioOwnedAdoptable")) { + $src = Get-FunctionSource -Path $setupPath -Name $fn + Check "setup.ps1 defines $fn" ($null -ne $src) + if ($src) { . ([scriptblock]::Create($src)) } +} + +# ── Source contract: the crash site must probe state, not bare Test-Path ── +$setupText = Get-Content -Raw -LiteralPath $setupPath +Check "prebuilt metadata probe no longer uses a bare Test-Path" ( + $setupText -notmatch '\n\s*if \(Test-Path \$existingMetaPath\)') +Check "prebuilt metadata probe goes through Get-PathState" ( + $setupText -match '\$existingMetaState = Get-PathState -Path \$existingMetaPath -PathType Leaf') +Check "a denied llama.cpp install fails with an actionable message" ( + $setupText -match '\$existingMetaState -eq "Denied"' -and + $setupText -match 'Exit-SetupFailure "Access denied reading the existing \$Label') +# Every denial route reports instead of proceeding: an unreadable parent dir, an +# unreadable metadata file, an unreadable .git checkout, and the ownership guard. +Check "the prebuilt phase stops on a denied llama.cpp dir" ( + $setupText -match '\$llamaDirState = Get-PathState -Path \$LlamaCppDir' -and + $setupText -match '\$llamaDirState -eq "Denied"') +Check "the source-build .git probe stops on a denied checkout" ( + $setupText -match '\$llamaGitState = Get-PathState -Path \(Join-Path \$LlamaCppDir "\.git"\)' -and + $setupText -match '\$llamaGitState -eq "Denied"') +Check "the ownership guard stops on a denied root instead of returning" ( + $setupText -match '\$pathState = Get-PathState -Path \$Path -PathType Container' -and + $setupText -match '\$StudioHomeIsCustom -and \$pathState -eq "Denied"') +Check "guidance says an app reinstall does not reset the folder" ( + $setupText -match 'reinstalling Unsloth Studio, to any drive, reuses it' -and + $setupText -match 'Reinstalling the app does not reset it\.') +Check "guidance names the concrete recovery commands" ( + $setupText -match 'takeown /F' -and $setupText -match 'icacls .* /reset /T') +Check "whisper marker probe cannot terminate the non-fatal whisper phase" ( + $setupText -match 'if \(Test-PathQuiet \$llamaMarker "Leaf"\)') + +# ── Behaviour against a real unreadable directory ── +$root = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_acl_" + [guid]::NewGuid().ToString("N")) +$locked = Join-Path $root "llama.cpp" +New-Item -ItemType Directory -Force -Path $locked | Out-Null +$meta = Join-Path $locked "UNSLOTH_PREBUILT_INFO.json" +Set-Content -LiteralPath $meta -Value '{"release_tag":"app-1","published_repo":"unslothai/llama.cpp"}' + +$onWindows = ($env:OS -eq "Windows_NT") +function Set-Denied([bool]$on) { + if ($onWindows) { + $who = "$env:USERDOMAIN\$env:USERNAME" + if ($on) { icacls $locked /deny "${who}:(OI)(CI)(RX)" *>$null } + else { icacls $locked /remove:d "$who" *>$null } + } else { + if ($on) { chmod 000 $locked } else { chmod 755 $locked } + } +} + +try { + Check "readable metadata reports Present" ((Get-PathState -Path $meta -PathType Leaf) -eq "Present") + Check "readable install is adoptable" (Test-StudioOwnedAdoptable $locked) + Check "absent path reports Absent" ( + (Get-PathState -Path (Join-Path $root "missing.json") -PathType Leaf) -eq "Absent") + + Set-Denied $true + + # Negative control AND environment gate: the old unguarded form must blow up + # here, otherwise this host cannot produce a denial (root / admin bypass) + # and the assertions below would pass vacuously. + $oldFormTerminated = $false + try { $null = Test-Path $meta } catch { $oldFormTerminated = $true } + + if (-not $oldFormTerminated) { + Write-Host " SKIP cannot deny access on this host (running as root/admin?) -- behaviour checks skipped" -ForegroundColor Yellow + } else { + Check "bare Test-Path still terminates on a denied path (negative control)" $oldFormTerminated + $state = $null + $threw = $false + try { $state = Get-PathState -Path $meta -PathType Leaf } catch { $threw = $true } + Check "Get-PathState does not terminate on a denied path" (-not $threw) + Check "Get-PathState reports Denied (not Absent)" ($state -eq "Denied") + + $quiet = $null + $threw = $false + try { $quiet = Test-PathQuiet $meta } catch { $threw = $true } + Check "Test-PathQuiet does not terminate on a denied path" (-not $threw) + Check "Test-PathQuiet reports the path as unusable" ($quiet -eq $false) + + $threw = $false + $adoptable = $null + try { $adoptable = Test-StudioOwnedAdoptable $locked } catch { $threw = $true } + Check "Test-StudioOwnedAdoptable does not terminate on a denied tree" (-not $threw) + Check "Test-StudioOwnedAdoptable cannot adopt an unreadable tree" ($adoptable -eq $false) + } +} finally { + Set-Denied $false + Remove-Item -Recurse -Force -LiteralPath $root -ErrorAction SilentlyContinue +} + +# ── Test-Path parity: the regression-safety invariant ── +# Wherever the old bare probe returned a value instead of throwing, Get-PathState +# must agree. Denied may only appear where the old probe threw, so no path that +# used to work can take a different branch now. +$parityRoot = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_par_" + [guid]::NewGuid().ToString("N")) +New-Item -ItemType Directory -Force -Path (Join-Path $parityRoot "tree/sub") | Out-Null +Set-Content -LiteralPath (Join-Path $parityRoot "tree/UNSLOTH_PREBUILT_INFO.json") -Value "{}" +Set-Content -LiteralPath (Join-Path $parityRoot "tree/sub/file.txt") -Value "x" +$parityProbes = @($parityRoot, (Join-Path $parityRoot "tree"), (Join-Path $parityRoot "tree/sub"), + (Join-Path $parityRoot "tree/UNSLOTH_PREBUILT_INFO.json"), (Join-Path $parityRoot "tree/sub/file.txt"), + (Join-Path $parityRoot "missing"), (Join-Path $parityRoot "missing/deeper.json")) +$mismatch = 0; $deniedWithoutThrow = 0; $probed = 0 +foreach ($p in $parityProbes) { + foreach ($t in @("Any", "Leaf", "Container")) { + $old = $null; $threw = $false + try { $old = [bool](Test-Path -LiteralPath $p -PathType $t -ErrorAction Stop) } catch { $threw = $true } + $new = Get-PathState -Path $p -PathType $t + $probed++ + if ($threw) { if ($new -notin @("Denied", "Absent")) { $mismatch++ } } + elseif ($new -eq "Denied") { $deniedWithoutThrow++ } + elseif ($old -ne ($new -eq "Present")) { $mismatch++ } + } +} +Remove-Item -Recurse -Force -LiteralPath $parityRoot -ErrorAction SilentlyContinue +Check "Get-PathState matches bare Test-Path on every non-throwing probe ($probed)" ($mismatch -eq 0) +Check "Denied never appears where the old probe did not throw" ($deniedWithoutThrow -eq 0) + +# ── A denied marker FILE under a readable directory ── +# Windows only: POSIX keeps a mode-000 file stat-able, so this state cannot be +# built on Unix. Collapsing it to "No" made the ownership guard report an +# Unsloth tree as an unrelated directory instead of a permissions problem. +$adoptRoot = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_adopt_" + [guid]::NewGuid().ToString("N")) +New-Item -ItemType Directory -Force -Path $adoptRoot | Out-Null +$adoptMarker = Join-Path $adoptRoot "UNSLOTH_PREBUILT_INFO.json" +Set-Content -LiteralPath $adoptMarker -Value '{"release_tag":"app-1"}' +Check "a readable marker reports Yes" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Yes") +if ($onWindows) { + $who = "$env:USERDOMAIN\$env:USERNAME" + icacls $adoptMarker /deny "${who}:(R)" *>$null + try { + $markerThrew = $false + try { $null = Test-Path -LiteralPath $adoptMarker -PathType Leaf -ErrorAction Stop } catch { $markerThrew = $true } + if ($markerThrew) { + Check "a denied marker reports Denied, not No" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Denied") + Check "the boolean view still refuses to adopt it" (-not (Test-StudioOwnedAdoptable $adoptRoot)) + } else { + Write-Host " SKIP this host would not deny the marker file" -ForegroundColor Yellow + } + } finally { icacls $adoptMarker /remove:d "$who" *>$null } +} else { + Write-Host " SKIP denied marker file is a Windows-ACL-only state (POSIX keeps mode-000 files stat-able)" -ForegroundColor Yellow +} +Remove-Item -Recurse -Force -LiteralPath $adoptRoot -ErrorAction SilentlyContinue +Check "a missing marker reports No" ((Get-StudioAdoptableState -Path ([System.IO.Path]::GetTempPath())) -eq "No") + +# ── The reporting path must not itself fail ── +# Get-PathDenialDetail runs while a failure is being reported, so a null or empty +# path must not replace the actionable message with a binding exception. +foreach ($edge in @($null, "", " ")) { + $edgeOk = $true + try { $null = Get-PathDenialDetail -Path $edge } catch { $edgeOk = $false } + Check "Get-PathDenialDetail tolerates an empty/null path" $edgeOk +} + +# ── The desktop app must receive the reason, not just "exit code 1" ── +# The real Exit-PathAccessDenied with the real Exit-SetupFailure in Tauri mode: +# install.rs prefers a [TAURI:ERROR] line over its generic exit-code message, so +# this is what the user reads. +$exitDeniedSrc = Get-FunctionSource -Path $setupPath -Name Exit-PathAccessDenied +$exitSetupSrc = Get-FunctionSource -Path $setupPath -Name Exit-SetupFailure +Check "setup.ps1 defines Exit-PathAccessDenied" ($null -ne $exitDeniedSrc) +if ($exitDeniedSrc) { + $harness = @" +`$ErrorActionPreference = "Stop" +function step { param([string]`$Label, [string]`$Value, [string]`$Color = "Green") Write-Host " `$Label `$Value" } +function substep { param([string]`$Message, [string]`$Color = "DarkGray") Write-Host " `$Message" } +function Get-PathDenialDetail { param([string]`$Path) return "" } +$exitSetupSrc +$exitDeniedSrc +Exit-PathAccessDenied -Path "C:\Users\test\.unsloth\llama.cpp" -Label "llama.cpp install" +Write-Host "REACHED_UNREACHABLE" +"@ + $harnessFile = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_denied_" + [guid]::NewGuid().ToString("N") + ".ps1") + Set-Content -LiteralPath $harnessFile -Value $harness -Encoding utf8 + $pwshExe = (Get-Command pwsh -ErrorAction SilentlyContinue).Source + if (-not $pwshExe) { $pwshExe = (Get-Command powershell).Source } + $savedMode = $env:UNSLOTH_TAURI_MODE + try { + $env:UNSLOTH_TAURI_MODE = "1" + $out = & $pwshExe -NoProfile -File $harnessFile 2>&1 | Out-String + $code = $LASTEXITCODE + } finally { + if ($null -eq $savedMode) { Remove-Item Env:UNSLOTH_TAURI_MODE -ErrorAction SilentlyContinue } + else { $env:UNSLOTH_TAURI_MODE = $savedMode } + Remove-Item -LiteralPath $harnessFile -ErrorAction SilentlyContinue + } + Check "the denial stops setup (exit 1)" ($code -eq 1) + Check "the denial does not fall through to the install" ($out -notmatch "REACHED_UNREACHABLE") + Check "the desktop app gets a [TAURI:ERROR] reason, not a bare exit code" ( + $out -match '\[TAURI:ERROR\] Access denied reading the existing llama\.cpp install') + Check "the reason names the folder to remove" ($out -match [regex]::Escape('C:\Users\test\.unsloth\llama.cpp')) + Check "the reason says a reinstall will not help" ($out -match 'Reinstalling the app does not reset it') + # takeown and icacls must be copy-pasteable. On one line, "then" is not a + # PowerShell separator and takeown would swallow the rest as arguments. + $takeownLines = @($out -split "`r?`n" | Where-Object { $_ -match 'takeown /F' }) + Check "takeown is printed on its own line" ($takeownLines.Count -eq 1) + Check "icacls is not appended to the takeown line" ( + $takeownLines.Count -eq 1 -and $takeownLines[0] -notmatch 'icacls') + Check "icacls is printed on its own line" ( + @($out -split "`r?`n" | Where-Object { $_ -match 'icacls .* /reset /T' }).Count -eq 1) +} + +# ── Denial classification ── +Check "UnauthorizedAccessException classifies as access denied" ( + Test-AccessDeniedError ([System.UnauthorizedAccessException]::new("denied"))) +Check "a wrapped UnauthorizedAccessException classifies as access denied" ( + Test-AccessDeniedError ([System.Exception]::new("outer", [System.UnauthorizedAccessException]::new("denied")))) +Check "an unrelated exception does not classify as access denied" ( + -not (Test-AccessDeniedError ([System.IO.FileNotFoundException]::new("missing")))) + +if ($script:failures -gt 0) { + Write-Host "$($script:failures) check(s) failed" -ForegroundColor Red + exit 1 +} +Write-Host "All checks passed" -ForegroundColor Green diff --git a/tests/studio/test_resolve_cuda_toolkit.ps1 b/tests/studio/test_resolve_cuda_toolkit.ps1 index 6be0adc621..c1114f7b38 100644 --- a/tests/studio/test_resolve_cuda_toolkit.ps1 +++ b/tests/studio/test_resolve_cuda_toolkit.ps1 @@ -31,6 +31,16 @@ $mismatchFn = $ast.FindAll({ param($n) if ($mismatchFn.Count -ne 1) { throw "expected exactly one Write-CudaDriverToolkitMismatch, found $($mismatchFn.Count)" } $mismatchText = $mismatchFn[0].Extent.Text +# -RequireOrExit leaves through Exit-SetupFailure, so the child needs the real +# one. Without it the call was an ignored command-not-found under +# ErrorActionPreference=Continue, the child fell through and exited 0, and the +# two "exits non-zero" checks failed on every run while CI stayed green. +$exitFn = $ast.FindAll({ param($n) + $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq "Exit-SetupFailure" +}, $true) +if ($exitFn.Count -ne 1) { throw "expected exactly one Exit-SetupFailure, found $($exitFn.Count)" } +$exitText = $exitFn[0].Extent.Text + # --- Spoof executables for driver/toolkit compatibility scenarios --- $work = Join-Path ([System.IO.Path]::GetTempPath()) ("rct_" + [guid]::NewGuid().ToString("N")) New-Item -ItemType Directory -Force -Path $work | Out-Null @@ -84,6 +94,8 @@ function Find-Nvcc { `$script:CudaToolkitReady = `$false `$script:NvccPath = `$null; `$script:CudaToolkitRoot = `$null; `$script:CudaArch = `$null +$exitText + $mismatchText $fnText diff --git a/tests/test_studio_install_workspace_guard.py b/tests/test_studio_install_workspace_guard.py index fa6c8afea4..aeb22cac45 100644 --- a/tests/test_studio_install_workspace_guard.py +++ b/tests/test_studio_install_workspace_guard.py @@ -253,7 +253,9 @@ def test_setup_ps1_prebuilt_llama_cpp_has_ownership_guard(): def test_setup_ps1_adopts_existing_whisper_prebuilt_marker(): text = SETUP_PS1.read_text(encoding = "utf-8") - helper_start = text.index("function Test-StudioOwnedAdoptable") + # The marker scan lives in Get-StudioAdoptableState; Test-StudioOwnedAdoptable + # is the boolean view of it. + helper_start = text.index("function Get-StudioAdoptableState") helper_end = text.index("function Assert-StudioOwnedOrAbsent", helper_start) helper = text[helper_start:helper_end] assert "UNSLOTH_WHISPER_PREBUILT_INFO.json" in helper @@ -379,7 +381,8 @@ def test_setup_helpers_gate_on_canonical_custom_root(): def test_setup_ps1_inplace_git_sync_marks_studio_owned(): """setup.ps1 in-place git-sync branch must Mark-StudioOwned after a successful sync.""" src = SETUP_PS1.read_text(encoding = "utf-8") - inplace_idx = src.index('Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")') + # Three-state probe so an ACL-denied tree stops instead of cloning over it. + inplace_idx = src.index('if ($llamaGitState -eq "Present") {') # The in-place branch ends just before the temp-dir clone branch. clone_idx = src.index("Cloning llama.cpp @", inplace_idx) inplace_block = src[inplace_idx:clone_idx] @@ -394,7 +397,8 @@ def test_setup_ps1_inplace_git_sync_marks_studio_owned(): def test_setup_ps1_inplace_git_sync_asserts_studio_owned_before_mutation(): """setup.ps1 in-place git-sync must Assert-StudioOwnedOrAbsent before any destructive git op.""" src = SETUP_PS1.read_text(encoding = "utf-8") - inplace_idx = src.index('Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")') + # Three-state probe so an ACL-denied tree stops instead of cloning over it. + inplace_idx = src.index('if ($llamaGitState -eq "Present") {') clone_idx = src.index("Cloning llama.cpp @", inplace_idx) inplace_block = src[inplace_idx:clone_idx] assert (