Commit graph

4 commits

Author SHA1 Message Date
Daniel Han
a151ac875c
Make install.ps1 work with the user's PowerShell profile loaded (#8161)
* Make install.ps1 work with the user's PowerShell profile loaded

Installing from a normal console failed where the same install from a
console started with -NoProfile succeeded. A profile runs before
`irm https://unsloth.ai/install.ps1 | iex` does and shares its scope, and
that entry point has no script file to re-launch without it, so the
individual couplings are cut instead.

install.ps1, at the top of Install-UnslothStudio:

- Set-StrictMode -Off. The script tests environment variables that are
  legitimately unset and reads $script: state only some branches assign,
  both of which a profile's `Set-StrictMode -Version Latest` turns into
  terminating errors.
- $PSDefaultParameterValues is filtered down to proxy keys. An entry like
  'Start-Process:WindowStyle' silently rebinds cmdlets here and fails the
  install with an error naming none of it. Proxy entries are kept because
  they can only ever enable a download, and on a locked-down host may be
  the only route to python.org and the uv release.
- $PSNativeCommandUseErrorActionPreference = $false. With a profile
  turning it on, the "Stop" preference makes a failing native command
  throw out of the `unsloth studio setup` handoff instead of reaching
  Exit-InstallFailure, skipping rollback and the Tauri error record.

All three assign without a scope qualifier, so they apply to the
installer and everything it calls and leave the caller's session alone.

uv is resolved once through Resolve-UvExecutable, which uses
`Get-Command uv -CommandType Application -All` and falls back to the bare
token when nothing is on PATH. PowerShell ranks aliases and functions
above PATH, so a profile `Set-Alias uv ...` was answering the version
probe and ending the install at "uv could not be installed" on machines
that had a working uv. Test-UvVersionOk pins the executable that
answered in $script:UvExe, and the 27 install scriptblocks invoke that
path. $script:UvExe and $script:UvInstallDestDir are reset per
invocation, since $script: is the caller's session under irm | iex.

unsloth_cli/commands/studio.py passes -NoProfile to setup.ps1
unconditionally. It was only added when stdout was not a tty, which is
never the case for the console install this fixes, so setup.ps1 ran
under the profile with its own bare uv calls exposed.

tests/test_installer_profile_hardening.py runs the extracted prologue and
uv probe under a hostile profile and checks the caller's session is left
intact. The four existing tests that anchored on the literal
`uv venv $VenvDir` are re-anchored past the command token.

* Plant the real-profile fixture where pwsh actually looks

test_a_real_profile_reproduces_the_same_state failed on ubuntu-latest with
every probed setting at its default, meaning the planted profile never loaded.
It passed here and on macos-14.

PowerShell resolves $PROFILE from $XDG_CONFIG_HOME when that is set and only
falls back to $HOME/.config when it is not, and GitHub's ubuntu image writes
XDG_CONFIG_HOME into /etc/environment. The fixture redirected HOME alone, so on
a hosted runner the inherited value went on naming the real account and the
profile was written to a path pwsh never opened. Setting XDG_CONFIG_HOME to the
same directory HOME already implies reproduces the CI failure exactly on this
machine, and removing it makes the failure go away.

_hostile_env now redirects XDG_CONFIG_HOME alongside HOME, so the two rules
agree whichever one the host applies, and the test asks pwsh for the path
instead of hardcoding the fallback branch. The two guards are precise rather
than blanket: a machine-wide profile loads into the real leg only, and a
$PROFILE that lands outside the fixture cannot be planted into. Neither is
reachable on Linux or macOS with the redirect in place.

The other tests in the file never shared the premise; every other pwsh launch
there passes -NoProfile and dot-sources the profile explicitly.

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

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

* Close the uv wrapper hole, harden winget the same way, and restore module autoloading

Validating the profile hardening against real pwsh turned up three things the
first pass missed. Six of the nineteen hostile-profile scenarios I exercised
are genuinely broken on main and genuinely fixed by this branch, so the shape
of the fix is right -- these are gaps in its coverage, not a change of
direction.

Resolve-UvExecutable still handed back the bare token when nothing named uv
was on PATH. That was meant to keep a working non-Application uv working, but
it reopens the exact hole the function exists to close: a profile
`function uv { Write-Output "uv 99.0.0" }` clears the version gate, gets
pinned into $script:UvExe, and then receives every install command the script
runs, with the user's torch, index URL and venv path as arguments. The
existing test missed it because its hostile alias reports no version at all,
and an alias to a missing file fails loudly. Follow an alias as far as an
Application and return that resolved path, since aliasing uv at a specific
build is a legitimate thing to do; return $null for anything else, which puts
the caller back on its install-uv branch and the gate re-probes against the
real thing.

winget had the identical defect and was left untouched. It is detected with a
bare Get-Command and invoked as a bare token at five sites, and it is what
installs both Python and uv -- so a `function winget` wrapper, which people
write to inject --accept-* or pin a source, owns the whole bootstrap. Same
treatment: resolve once to an Application and invoke through the path.

A profile setting $PSModuleAutoLoadingPreference to 'None' is fatal here and
was not covered. PowerShell 7 loads no modules at startup, so that one line
removes Test-Path, Write-Host, Select-Object, ConvertFrom-Json, Get-FileHash,
Invoke-WebRequest, Expand-Archive, Start-Process and Get-Content, and the
script dies on its first step naming a cmdlet the reader assumes is always
there. Windows PowerShell 5.1 preloads Utility and Management and survives,
which is exactly what makes this reproduce on one machine and not another.

Also: use [regex]::IsMatch in the defaults filter so it leaves no $Matches
behind; add -NoProfile unconditionally in _refresh_desktop_shortcuts, which
launches install.ps1 and had it gated on the hidden branch, so the visible
console path -- the one where a profile IS loaded -- was the one that missed
it; and record that the preserved proxy defaults do not reach setup.ps1,
which is launched with -NoProfile, along with why that trade is accepted.

Tests: the assertion that the bare token must come back now asserts the
opposite, and there are new ones for a convincing uv function, an alias to a
real uv, the winget call sites and the autoloading reset. The two new pwsh
tests execute against a genuinely planted profile rather than reading source.

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

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

* Carry the profile proxy across the setup handoff, not just inside install.ps1

install.ps1 deliberately keeps proxy-shaped $PSDefaultParameterValues entries out of the
profile table it discards, because on a locked-down corporate host that entry can be the
only route out. Adding -NoProfile to the setup launch unconditionally then threw them away
one process later, and setup.ps1 downloads on its own: the VC++ runtime through
Invoke-WebRequest and the uv installer through Invoke-RestMethod.

A PowerShell variable does not cross a process boundary, so the kept entries travel as
JSON in _UNSLOTH_PS_PROXY_DEFAULTS and the child re-applies them before running setup.ps1.
Nothing else from the profile comes with them. A credential is left behind on purpose:
PSCredential does not survive ConvertTo-Json, and the environment is the wrong place for
one. A stale variable is cleared when there is nothing to hand off.

Three tests, one static and two driving real pwsh, covering the round trip, the credential
and non-proxy keys being dropped, and the prelude staying silent when the variable is
absent, empty or corrupt.

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

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

* Harden the proxy handoff: ordering, key casing, uri values, scope, and the standalone update

Five follow-ups, all on the handoff added last round.

The handoff serializes with ConvertTo-Json, from Microsoft.PowerShell.Utility, and ran before
the module-autoloading reset. Under a profile's $PSModuleAutoLoadingPreference = 'None' a fresh
PowerShell 7 session therefore died right there, taking out the one configuration the handoff
exists to support. The reset moves to the front of the prologue.

The key filter was a case-sensitive .NET regex, but cmdlet and parameter names bind
case-insensitively, so 'invoke-webrequest:proxy' was dropped. And [uri] is the type the Proxy
parameter actually takes, so a careful profile assigns one; the serializer accepted only string
and bool, and it disappeared at the process boundary. Both are accepted now, a uri by its
AbsoluteUri. A PSCredential is still deliberately left behind.

Under "irm ... | iex" the prologue runs in the caller's own session, so writing the environment
variable there outlived the install on every path, early returns included, and a later
`unsloth studio update` from that console would reapply stale JSON over a proxy that had since
changed. The prologue now holds the value and it is published around the setup child only,
saved and restored beside the other child-scoped variables.

A standalone `unsloth studio update` has no installer above it, so there was nothing to
restore and -NoProfile left it with no route out. It now asks: a throwaway PowerShell that does
load the profile prints just the proxy-shaped defaults as JSON, validated before use, entirely
best effort. Same filter as install.ps1's.

Five tests, two driving real pwsh, including one against a profile with strict mode on,
autoloading off, a lowercase key and a uri value.

* Ask the profile the caller actually has, and follow a uv alias first

- the standalone update probed powershell.exe only, so a proxy living in the
  PowerShell 7 profile never reached the -NoProfile child; both editions are
  asked now, the caller's first, and their answers merged.
- Resolve-UvExecutable checked PATH before the alias, which is the reverse of
  PowerShell's own resolution and made the alias branch unreachable on any
  machine with some uv on PATH.
- the parity workflow did not run this suite when unsloth_cli/commands/studio.py
  changed, though the suite asserts that module directly. Its own path-filter
  parser also treated a comment inside the list as the end of it, which would
  have hidden the addition.

* Give the parity job the imports it needs, and fold proxy keys the way PowerShell does

Three tests in the profile-hardening suite import unsloth_cli.commands.studio to drive
the profile probe directly, and that pulls typer, pyyaml, pydantic and click. The job
installed pip and pytest only, so on a clean setup-python both matrix legs died with
ModuleNotFoundError before a single test ran. Installed, with a test that keeps the step
in step with what the suite imports.

$PSDefaultParameterValues keys are case-insensitive and a Python dict is not, so
"Invoke-WebRequest:Proxy" from the caller's own host and "invoke-webrequest:proxy" from
the other one both crossed over; the prelude then replayed them in order and the
lower-priority host's value landed last, reversing the earlier-host-wins rule this merge
exists for. Keys are folded now, first spelling seen wins, within one profile's answer
as well as across two.

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

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

* Frame the proxy record, and quote the union so 3.9 can still import the CLI

The probe runs after the profile, and the profile is free to print: a MOTD, a "loading
personal and system profiles took 812ms" line, a corporate banner. With the record bare,
that arrived ahead of the JSON, the parse threw and the whole answer was dropped -- so
the locked-down host that needed the proxy handed the -NoProfile child nothing and every
download failed, which is worse than before, since the old visible-console path loaded
the profile itself. The record is emitted between two markers now and cut out of
whatever else was said.

And `str | list[str]` was evaluated at def time in a module with no postponed
annotations, so on the 3.9 this project still supports it raised TypeError and took the
whole CLI import with it. Quoted, with a test that walks every annotation in the module
for an unquoted PEP 604 union.

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

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

* Let the uv gate move past a stale alias, and decode the probe output lossily

An alias pointing at a real but stale uv was the only binary the version gate ever
probed, so a current uv already on PATH -- or one winget or the pinned release had just
installed -- could not rescue the run and the install ended at "uv could not be
installed" on a machine that had one. The resolver hands back every candidate in the
order the bare token would pick them, alias first, and the gate walks them until one
passes, pinning the one that answered.

The profile probe decoded its child with text=True alone, which is the locale codec with
STRICT errors. A UTF-8 banner on an ANSI console then raised UnicodeDecodeError, which is
neither OSError nor SubprocessError, so it escaped the handler and took the update down
before the -NoProfile child ever ran -- and before the framing could discard the banner.
UTF-8 with replacement now; the record itself is ASCII.

rich is named in the parity job's install line too. It arrives through typer today, but
unsloth_cli imports it directly, and this suite's imports should not rest on somebody
else's dependency list.

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

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

* Assert the proxy handoff by parsing it, not by substring

CodeQL reads the bare membership test as an incomplete URL sanitization, which
is a fair reading of the shape even though this is an assertion on a compressed
JSON payload rather than a check on untrusted input. Parsing it and comparing
the value exactly is the stronger assertion anyway.

* Read the caller edition by order, pin the probe's encoding, claim cmdlets whole

A machine can carry both PowerShell module trees on PSModulePath at once, so
inferring the caller from the absence of the other edition handed precedence to
the wrong profile and let its proxy override the console the command was typed
into. Each host puts its own module directory first, so the earliest tree names
the caller; neither present keeps the previous order.

Windows PowerShell 5.1 writes redirected output in the console code page while
this process decodes UTF-8, so a non-ASCII proxy value came back with
replacement characters, still parsed as JSON, and handed setup a proxy that does
not resolve. The probe pins its own output encoding first.

And the merge claims a cmdlet whole rather than filling missing companion
parameters from the other profile, which built a configuration neither host had
-- one profile's proxy with the other's credential forwarding.

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

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

* Evaluate script-block proxy defaults, read the caller's host profile, drop the secret

A profile can set a dynamic default as a script block, which is PowerShell's
supported form and which Invoke-WebRequest evaluates per call. Both
serializers dropped it, so the caller downloaded fine and the -NoProfile
setup child got no proxy at all. Both now invoke the block and hand over the
resulting URI or string; executable code does not cross the handoff.

The probe spawns pwsh.exe or powershell.exe, which load the CONSOLEHOST
profile. A caller in the VS Code Integrated Console or the ISE keeps its
defaults in Microsoft.VSCode_profile.ps1 or Microsoft.PowerShellISE_profile.ps1
instead, so the probe reported no proxy on exactly the host that needed one.
It dot-sources the caller's other CurrentUser host profiles, from their own
directory, before reading the table.

And the prelude clears _UNSLOTH_PS_PROXY_DEFAULTS the moment it has read it.
A profile proxy routinely carries credentials, and every native process
setup.ps1 starts inherited the environment it was launched with.

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

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

* Probe only the caller's own host profile, on one shared budget

Sourcing every Microsoft.*_profile.ps1 in the profile directory ran profiles
belonging to hosts nobody was using: they can overwrite the console's own
$PSDefaultParameterValues, have side effects, or exit before the framed
record is written. The probe now sources exactly one, named by
_UNSLOTH_PS_HOST_PROFILE, and only when the caller identifies itself (VS Code
does, via TERM_PROGRAM). A host we cannot name gets no extra profile rather
than someone else's.

install.ps1 removed the handoff variable when it had no proxy to pass, and
its absence is precisely how the CLI recognises a standalone update -- so an
installer launch, including one started with -NoProfile or by the desktop
app, went and reloaded the profiles it had deliberately discarded. It
publishes an explicit empty handoff instead, and the CLI keys on presence.

And the probe's timeout is one budget for the whole call rather than one per
host, so two installed editions with two hung profiles no longer cost twice
the documented best-effort delay before setup starts.

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

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

* Run the proxy probe with -NoProfile and dot-source the caller's own two

Without -NoProfile the probe host loaded its own ConsoleHost profile before
the script ran, so an unrelated profile could print, rewrite
$PSDefaultParameterValues or exit before the record was written -- and it
still was not the profile a VS Code caller keeps its defaults in.

The child runs with -NoProfile now and dot-sources exactly the two the
caller's session would have loaded: $PROFILE.CurrentUserAllHosts and either
the host profile named in _UNSLOTH_PS_HOST_PROFILE or
$PROFILE.CurrentUserCurrentHost. $PROFILE is fully populated under
-NoProfile, since the paths are computed rather than loaded, so this is exact
rather than incidental.

Checked against pwsh with a fixture profile directory: a VS Code caller picks
up its own profile plus the all-hosts one and never runs the console
profile's banner, and a plain console caller picks up the console profile
plus the all-hosts one.

* Probe the all-users profiles too, and clear profile defaults before emitting

A machine-managed proxy commonly lives in AllUsersAllHosts on a domain-joined
box while the user's own profile never mentions it, so sourcing only the
current-user pair reported no proxy on exactly the host that has one. The probe
now walks PowerShell's own startup order, all-users first, so the user's profile
still gets the last word.

The profile's $PSDefaultParameterValues was also still active when the record
was serialized. ConvertTo-Json:AsArray = $true is a legitimate setting and turns
the payload into a JSON array, which the reader rejects for not being a
dictionary. $out already holds copies by then, so the table is cleared first.

* Harden the proxy probe against profile overrides, and drop the handoff copy

Five fixes from the review round:

install.ps1 kept the serialized proxy defaults in $script:, which under the
documented irm | iex path IS the caller's session scope, so an authenticated
proxy URI stayed readable in that console after the installer returned. Cleared
in the same finally that restores the environment handoff.

A profile setting [Console]::OutputEncoding overrode the probe's UTF-8 pin, and
the parent decodes that stream as UTF-8, so the framed record could come back
corrupted. Re-pinned after the last profile is sourced.

The record was emitted through bare Write-Output and ConvertTo-Json, which a
profile alias or function shadows; clearing $PSDefaultParameterValues does not
cover a command override. Both are module-qualified now.

TERM_PROGRAM=vscode is set by every VS Code integrated terminal, not only the
PowerShell extension's host, so substituting Microsoft.VSCode_profile.ps1 for
the current-host profile missed the proxy a plain pwsh terminal there actually
has. The named host profile is added rather than substituted, with the
current-host profile last.

The per-cmdlet ownership check compared command strings literally, so a wildcard
key from one host and a literal key for a matching cmdlet from the other were
both merged, which is how one invocation ends up configured from two profiles.
Overlap is matched in either direction now.

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

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

* Hold the proxy handoff in the frame, and treat two wildcards as one family

Three fixes from the review round:

The serialized handoff lived in $script:, which under the documented irm | iex
path is the caller's session scope, and the only cleanup ran after the setup
child. Dozens of exits return earlier -- ShortcutsOnly, an argument error, lock
contention, a failed dependency install -- so an authenticated proxy URI stayed
readable in that console. It is a function-local now, which dies with the frame
on every path including a throw.

install.ps1 serialized that record through a bare ConvertTo-Json, which a
profile alias or function shadows exactly as it does in the probe. Module
qualified.

The cmdlet-ownership check compared two wildcard patterns as strings, and
Invoke-Web* and *-WebRequest both apply to Invoke-WebRequest while neither
matches the other. Two patterns are now assumed to overlap: the cost is a second
host's unrelated wildcard entry going unmerged, against handing setup a
credential setting from a profile that never asked for one.

* Tighten the profile-hardening comments

Comments, docstrings and whitespace only; no code changes. Each comment keeps
the reason it records and drops the retelling.

* Cut the profile-hardening comments down again

Comments, docstrings and whitespace only. The install.ps1 prologue and the
studio.py proxy probe kept one causal claim per decision, with the probe's
profile-loading essay split into a short note beside each line it justifies.

* Pin the probe's add-both profile order in its own test name

* Keep disjoint wildcard proxy families, and give each probed host its own module path

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

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

---------

Co-authored-by: Daniel Han <moonshotaisubstack@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-09 04:27:28 -07:00
Daniel Han
07df95079e
Studio: route every Windows installer line through the UTF-8 stdout sink (#8148)
* Studio: route every Windows setup line through the UTF-8 stdout sink

The desktop setup log rendered "?? Unsloth Studio Setup" over a rule of
replacement characters. Tauri spawns Windows PowerShell 5.1 with
CREATE_NO_WINDOW (install.rs), so the [Console]::OutputEncoding setter
throws and both entry scripts rebind [Console]::Out to a UTF-8 writer.
step/substep already write only through that writer when stdout is
redirected, so they came out right. Every other line did not: Write-Host
is written by 5.1's console host with its own writer on the OEM code
page, and U+1F9A5 has no OEM form while U+2500 becomes a bare 0xC4, which
from_utf8_lossy turns into U+FFFD. The banner and the footer are not
steps, so they kept arriving as mojibake, and install.ps1 had neither the
IsOutputRedirected probe nor a mirror at all.

Add Write-StudioLine above the first write in studio/setup.ps1 and
install.ps1: console handle when redirected, Write-Host when interactive,
since it is the only writer that colorizes. Rewrite 164 call sites in
setup.ps1 and 155 in install.ps1 onto it, including install.ps1's own
step/substep. Write-Host now survives only inside helpers that have
already ruled out the redirected sink, and the launcher script install.ps1
generates keeps its own, since it runs as a separate process.

No behaviour change for an interactive console user: same text, same
colors, same single record per line.

test_windows_setup_output_encoding.py gains byte-level coverage that the
real banner and footer, sliced out of setup.ps1, survive both launch
shapes as valid UTF-8 exactly once, plus a source contract that runs on
Linux and names any file:line that reaches for Write-Host outside the
allow-list. Studio.Setup.Output.Tests.ps1 covers Write-StudioLine in both
modes and pins install.ps1's copy to setup.ps1's.

Harnesses that splice these scripts apart now stub or dot-source
Write-StudioLine: two PowerShell harnesses, one Python harness, and the
VC++ redist leg of studio-windows-inference-smoke.

pytest tests/python tests/test_installer_*.py: 1077 passed (2 pre-existing
sandbox failures unrelated to this change). All 16 tests/studio harnesses
and 57 Pester cases pass. Both scripts parse clean.

* CI: spawn install.ps1 as a child process so its lines reach install.log

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

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

* Stub the output sink in the llama.cpp backend PowerShell harness

* Guard the console-less spawn on a Windows runner

The byte-level cases in this file run with a console attached, and a GitHub
runner gives a CREATE_NO_WINDOW child one, so the UTF-8 setter succeeds there
and every version of these scripts emits a clean banner. Those cases cannot
tell this fix from what preceded it.

Add cases that call FreeConsole() in the child first, which is the state
install.rs's own comment assumes CREATE_NO_WINDOW produces. There Write-Host
has no screen buffer to query, throws, and takes the script down: 2 bytes of
stdout and exit 1 rather than the banner. The probe is assembled entirely out
of text sliced from the script under test and spawned with install.rs's own
interpreter, flags and creation flags.

No Windows job ran this file, so its byte-level half was only ever exercised
under pwsh 7 on the Linux Backend CI leg, which is UTF-8 by default. Add it to
the cross-platform parity matrix, which already has a windows-latest row and
already triggers on install.ps1 and studio/setup.ps1.

* Report skips in the parity step

A platform-gated case that stopped running on the row it exists for still
reports green with -q alone.

* Slice the error preference too

It is what turns the Write-Host throw into a dead script rather than a
skipped line, so restating it would be assuming the result.

* Say what the comments actually mean

* Make the console-less cases fail on a lost banner, not just a mangled one

* Stub the output sink in every harness that splices these scripts

The Write-Host rewrite left four spliced-source harnesses reaching
Write-StudioLine without defining it. An undefined command is a terminating
error, so each one either aborted or was swallowed by the harness's own catch,
and the test kept passing while no longer testing anything.

- test_windows_python_venv_hardening.py, partial-rollback case: the five-line
  split-move warning was lost. The assertion that "both halves are named" only
  stayed green because $existing is a prefix of the rollback dir, so it matched
  the dir= line instead. Pin it to the warning text.
- test_path_probe_access_denied.ps1, ownership guard: the catch scored the
  command-not-found as the intended failure and never reached Exit-SetupFailure.
  Pin the check to the EXIT-SETUP message.
- test_windows_installer_concurrency_guard.py: the decision block prints before
  Exit-InstallFailure, so on Windows the active case aborted at exit 1 and never
  produced RESULT:blocked.
- Studio.Setup.Vs2026.Tests.ps1: on a host without cmake,
  Ensure-BuildToolsForLlamaSourceBuild hits the sink first and the no-op case
  fails on the throw.

Also stub the three remaining harnesses that splice sink-calling helpers but do
not reach the sink on the paths they exercise today, so the next case added to
them cannot reintroduce this.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-08 06:47:03 -07:00
Tai An
6ea2b91d04
fix(studio/install): keep the rollback reference when a venv move stops partway (#7810) (#7823)
* fix(studio/install): keep the rollback reference when a venv move stops partway (#7810)

Start-StudioVenvRollback moves the existing environment aside with a single
Move-Item and treats any failure as "the original is still in place". On
Windows that is not the shape the failure takes: an open handle inside the
tree -- a running Studio backend holds its own unsloth.exe there -- fails the
rename *after* it has already walked part of it, so the entries handled
before the locked one sit at the rollback path while the rest stay behind.

Both paths then exist, and the catch tests only the source, so it scores the
split tree as an untouched original, clears StudioVenvRollbackDir and drops
the sole record of where the other half went. The reporter was left with a
unsloth_studio\ holding 7 Scripts entries and no python.exe, an intact venv
under unsloth_studio.rollback.<stamp>.<pid>\, and no output naming either.
Retries cannot recover: the create branch keys off python.exe, and uv refuses
to build a venv over the directory the stranded files still occupy.

Clear the rollback state only when the destination is genuinely absent. When
both paths exist the move is partial, so keep it active -- the existing
finally-block Restore-StudioVenvRollback then reverses it -- and print both
locations plus the "close Unsloth Studio" hint the launcher-shim path at
install.ps1:3078 already gives for the same underlying cause.

install.sh is unaffected: POSIX rename ignores open descriptors and both
paths are siblings under , so that move really is atomic.

Regression test extracts the function from install.ps1 and runs it under real
PowerShell with Move-Item stubbed to fail after creating the destination. On
current main the partial case reports active=False with an empty rollback dir
-- the stranding itself; the clean-failure case is asserted alongside it so
the untouched-original path keeps clearing state as before.

Signed-off-by: Tai An <antai12232931@outlook.com>

* fix(studio/install): merge a split venv back instead of clearing the target

Keeping the rollback active after a partway move sent the failure path into
Restore-StudioVenvRollback, which removes $target before moving $backup back.
In the split case $target is not an incomplete *new* environment -- it holds the
half of the previous one the move never reached -- so that removal deleted files
present nowhere else and restored a corrupted venv.

Flag the split and give restoration a merge path: move each entry of the backup
into the target without overwriting, recursing where the move stopped inside a
subtree, and only drop the backup once it is empty. Anything ambiguous is left
in place and both locations are named.

Regression test pins the file that never moved to surviving restoration.

* Installer: fix split-move merge nesting siblings and walking through junctions

Two problems in Merge-StudioVenvRollbackTree, both only reachable once a partway
move leaves the venv split.

Sibling nesting. The per-entry variable was named $destination, and PowerShell
variable names are case-insensitive, so it reassigned the $Destination parameter.
Only the first entry at a level landed correctly; every later sibling was joined
onto the previous one's path, so a restored venv came back with pyvenv.cfg inside
Lib. Renamed to $entryTarget.

Junction traversal. Recursion keyed on "directory on both sides", which a junction
or directory symlink satisfies. If the half left behind holds the link, venv files
move through it and land outside $StudioHome. If the moved half holds it, the
recursion enumerates the link target and pulls those files into the venv, emptying
a directory that was never part of the environment. Either way the link is replaced
by a real directory, which the whole-tree Move-Item this path replaced never did.
Now checks both sides for a reparse point and falls through to keep-both-copies.

Attributes are read via Get-Item on both sides, since Get-ChildItem has reported
them inconsistently.

Tests cover sibling placement and both link directions. The link test uses a
junction on Windows so it does not need SeCreateSymbolicLinkPrivilege.

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

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

---------

Signed-off-by: Tai An <antai12232931@outlook.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-06 02:09:52 -07:00
Etherl
1770182b5a
Windows: validate managed Python before package installation (#7763)
* Harden Windows pyenv interpreter handling

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

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

* Fix Windows managed Python recovery

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

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

* Tighten-installer-comments

* Finish the -S probe hardening and run the new tests on Windows for PR #7763

Get-PythonPlatformTag still probed without -S. Its result is compared with
-eq "win-amd64", so a sitecustomize banner reads as "unknown", the
x64-over-ARM64 preference is lost and Windows on ARM settles for a native
ARM64 interpreter. Test-IsCondaPython gets -S for the same reason. Neither
query needs site, and base_prefix and get_platform() are unchanged by -S on
3.11, 3.12 and 3.13.

The new test module parametrizes over pwsh and powershell, but
cross-platform-parity-ci.yml is the only three-OS job and its paths filter
and pytest list are hardcoded, so the 5.1 leg never ran. Added the file to
both.

Also match the py launcher branch to the PATH branch with -LiteralPath
-PathType Leaf, and fix the failure message: the empty base home leaked into
the Exit-InstallFailure text, and the ownership marker is written before the
gate, so a plain re-run already replaces the environment.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>
2026-08-03 05:08:47 -07:00