14 commits

Author	SHA1	Message	Date
Daniel Han	50b9deacb0	Trim comments in tests (#13844) ... Comment and docstring changes only, in test files, split out of #12968 so the test-only part can merge first.	2026-10-15 05:30:27 -07:00
Michael Han	3bf6002c69	Studio: list at most the 12 most recently active projects and sections in Move to (#12155) ... * Studio: list at most the 12 most recent projects and sections in Move to Move to rendered every project and every section, so a long list made the submenu slow to open. Each group now lists the 12 most recent: projects by last update, sections newest first as the store keeps them. The one the chat is already in is still left out. * Studio: rank Move to projects and sections by activity, not edit time or drag order A project's updatedAt only moves when it is edited, so a project with the newest chat could miss the 12. Rank by its newest chat, the same activity the sidebar's folder order uses, now one shared projectActivityAt. Section order follows drags, so the first 12 were not the most recent. New sections record createdAt, and a section ranks by the newest of that, its chats and its projects. * Apply required pre-commit formatting to merged main tests --------- Co-authored-by: wasimysaid <wasimysdev@gmail.com>	2026-09-28 01:46:05 -07:00
Daniel Han	be9e6afd92	Tests: stop Windows tests tripping Bitdefender and the 16-bit application dialog on real machines (#12167) ... * Keep Windows test runs free of AV hits and 16-bit dialogs The console-less output-encoding probe detached itself with a kernel32 FreeConsole P/Invoke written into the .ps1, which Bitdefender quarantines. Spawn the child with DETACHED_PROCESS instead so it never has a console, and have the probe report whether it can reach a console screen buffer so a run that still had one is refused rather than passing blind. The --no-rollback XPU case wrote batch text into Scripts\python.exe and let install.ps1 execute it, which raises the Unsupported 16-Bit Application dialog on Windows. Stub Invoke-BoundedPythonProbe there with the real helper's return shape, pin what it was asked, restore the extracted definition afterwards, and assert the verdict on Windows instead of skipping it. * Answer the launcher --version check without starting the MZ stubs Two restore tests left subprocess.run unpatched, so the updater ran the few-byte MZ launcher stubs for real. On a Windows desktop that can raise the 16-bit application dialog. Raise the exec format error those runs produced instead, and check which file the restore asked. * Answer non-PE launches on Windows instead of starting them The broken uv.exe in the FindInstalledUv Pester suite and the rotten llama-server.exe in the keep-install back-compat test are text files that the code under test really launched on Windows, where a .exe that is not a PE image can raise the 16-bit application dialog. On Windows only, Start-Process (Pester) and run_capture (pytest) now refuse a file without an MZ header with ERROR_BAD_EXE_FORMAT, the answer the loader gives, and never start it. Real images still launch for real and POSIX keeps the real exec. The uv suite also checks the broken file is not an image and that the finder did ask it before skipping. * Keep the in-child FreeConsole probe, since 5.1 cannot start detached, and run it only on CI or opt-in * Drop the RawUI console check; the host keeps its screen buffer handle after FreeConsole * Stand in for real .exe files with a pip-style console launcher instead of a renamed where.exe	2026-09-28 00:16:41 -07:00
Daniel Han	bd612c8635	Remove the reflection-emit apparatus and all four emitted types (#11193)	2026-09-27 21:47:59 -07:00
Daniel Han	4becbbd24d	Delete the emit apparatus that existed to colour a banner, now that measurement says it was a no-op (#10999) ... * Windows installers: read the console's virtual terminal support instead of emitting a P/Invoke thunk for it install.ps1 and studio/setup.ps1 both emitted a reflection-built type declaring GetStdHandle, GetConsoleMode and SetConsoleMode for one consumer: the ANSI colour banner. That cluster (a dynamic assembly, a kernel32 P/Invoke, and in setup.ps1 a hidden child interpreter to gate it) is the densest malicious-looking region in either script and the only thing behind the one high-severity Sigma rule VirusTotal reports, and it is decoration. Windows PowerShell's console host performs that exact sequence at startup and reports the outcome through $Host.UI.SupportsVirtualTerminal. The pre-flight in PR #10984 measured 5.1.26100 attached to a real console: the property answers True, the native call answers True, and the console mode read before touching it is already 0x7, so the SetConsoleMode was re-setting a bit the host had set. The property is also the stricter of the two, because TryTurnOnVirtualTerminal re-reads the mode after setting it while the deleted code trusted SetConsoleMode's return value. The redirected case is unchanged and still decided first, by $script:StudioStdoutRedirected, before the property is read, so the Tauri pipe path and the CLI are untouched. The emit apparatus itself stays in both files. setup.ps1's Get-NvidiaLibraryProbeType emits the nvml and nvcuda imports through the same gate, so its coverage in tests/python/test_windows_installer_native_type_fallback.py stays parametrised over both scripts; only the console-helper test is install-only. Rebuilt as one commit on current main. * [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>	2026-09-15 14:04:20 -07:00
Daniel Han	95feb6979e	Windows installer: emit the native path helper instead of compiling it (#10540) ... * Windows installer: emit the native path helper instead of compiling it Bitdefender blocks Unsloth Desktop during the first "Get started" run on Windows 11, including elevated. The chain it reports is unsloth-stud... -> powershell.exe -> csc.exe -> %TEMP%\<random>.dll and the verdict lands on the generated DLL (Gen:Variant.MSILHeracles), not on anything we ship. Bisecting the reports: 0.1.800-beta installs fine, 0.1.803-beta and later block. In that window the installer went from one Add-Type compile to two, the second retrying under a private TEMP after the first failed. Every shipped exe in the range is 0/70 on VirusTotal, so the shipped bytes are not what is being detected; the behaviour is. Add-Type -TypeDefinition on Windows PowerShell 5.1 has no in-process compiler, so it writes C# to %TEMP% and shells out to csc.exe to produce a DLL it then loads. PowerShell launching a compiler that emits executable code into %TEMP% is a generic malware pattern, and heuristics fire on it. The only thing the installer needed from that compile was three kernel32 P/Invoke declarations used to resolve a final path across a reparse point. Reflection.Emit builds exactly those, in memory: DefineDynamicAssembly with AssemblyBuilderAccess::Run, then DefinePInvokeMethod per import. No compiler process, no source file, no DLL on disk, and the assembly reports an empty Location. Available on .NET Framework 4 and on .NET 5+, so 5.1 and 7 take the same path. Changes: - Initialize-StudioFinalPathNativeType emits CreateFileW, GetFinalPathNameByHandleW and CloseHandle via DefinePInvokeMethod. - Drop the private-TEMP retry added for the earlier compile failure. There is no compiler to fail, and that retry is the second csc.exe launch in the chain above. - Get-StudioNativeFinalPath is now non-throwing end to end. DefinePInvoke cannot request SetLastError, so Marshal.GetLastWin32Error is not meaningful here; a failed call returns null and the caller drops to the existing lexical answer with -Exact false, exactly as before. - Get-StudioProcessImagePath loses its native branch. OpenProcess and QueryFullProcessImageNameW existed only to shortcut what Get-Process and Win32_Process already answer, and both were already the fallback. install.ps1 now contains zero Add-Type -TypeDefinition sites. Tests: - tests/studio/test_native_path_resolver_degrades.ps1 runs the real extracted resolver under pwsh on POSIX, where kernel32 does not resolve, which is the same state as a host whose helper was taken away. It asserts the type defines, nothing is written to the temporary directory, the assembly is memory-only, and a native call that cannot work returns null rather than throwing. - test_installer_av_shapes.py learns to read imports declared through DefinePInvokeMethod, asserts no C# compiler is invoked, and keeps asserting the lexical fallback is still reachable. Behaviour is unchanged on a machine where the native call works: same exact path, same -Exact true. On a machine where it does not, the caller already handled null and fails closed on the runtime lock as before. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: the remaining compiles, the process rung, and the code integrity gate Review of the first commit turned up three things. 1. install.ps1 still compiled C# twice. Add-Type -MemberDefinition reaches csc.exe exactly as -TypeDefinition does: it wraps its argument in a class and compiles that. Two sites were left, the virtual-terminal console thunk and the Explorer icon refresh, and the argument for leaving them was conditional: the redirect guard keeps the desktop app off the first, and Tauri returns before the second. A guard that holds only for the entrypoint we happened to look at is what let this reach the field in the first place, and the standalone `irm | iex` path reached the console compile on every run. Both now emit. The three emit sites share one helper, New-StudioEmittedNativeType, and studio/setup.ps1 carries its own copy for its own console thunk, which was compiling too. Add-Type no longer appears in either script. -AssemblyName would be the one acceptable form, since it loads an assembly that already exists and reaches no compiler, but neither script uses it outside a generated launcher. 2. Dropping the native process-image rung was a regression. The first commit removed OpenProcess and QueryFullProcessImageNameW from Get-StudioProcessImagePath on the grounds that Get-Process and Win32_Process already answer. They do not always. Process.Path goes through MainModule, which needs PROCESS_VM_READ and is refused across users and across bitness, while QueryFullProcessImageNameW needs only PROCESS_QUERY_LIMITED_INFORMATION and Win32_Process needs a working WMI service. With neither answering, Get-RunningStudioVenvProcesses finds nothing and the install proceeds over a venv Unsloth still has open, which is the case its abort exists for. The rung is back, emitted rather than compiled. The ladder is now identical to the one on main. 3. Reflection emit has one failure mode that cannot be caught. App Control's Dynamic Code Security (policy option 19) always blocks loading unsigned assemblies built with System.Reflection.Emit, and Microsoft documents the parent process as usually stopped or crashing rather than raising, so a try/catch around the emit is not enough: learn.microsoft.com/en-us/windows/security/application-security/application-control/app-control-for-business/design/appcontrol-and-dotnet Test-StudioCanDefineNativeTypes is now the gate every emit site checks first. It keeps the Constrained Language check, which is what fires in practice since a host enforcing user-mode code integrity puts PowerShell in Constrained Language, and adds a Win32_DeviceGuard probe for a policy that somehow left the language mode alone. Unreadable means unrestricted: a host with no Device Guard at all is the common case and is not a reason to refuse. Also: stale comments describing the compile, the CS2001 failure and the private TEMP retry, none of which exist any more. Tests: - tests/studio/test_native_path_resolver_degrades.ps1 now parses install.ps1 with the PowerShell parser rather than the brace-counting extractor in tests/studio_setup_ps1, which runs past the end of any function holding a brace inside a string; Resolve-StudioLinkTarget has one. It runs the whole ladder, not only its top rung, and asserts the lexical answer still arrives with Exact false, that both process-image rungs answer, that GetConsoleMode's out parameter is emitted by-ref and SHChangeNotify still returns void, and that defining a type twice neither throws nor corrupts the first definition. - test_installer_av_shapes.py bans Add-Type in both scripts rather than -TypeDefinition in one, and OpenProcess and QueryFullProcessImageNameW are back in the allowed import inventory with the reason they cannot be replaced. tests/studio plus the Windows installer guards: 8688 passed, 215 skipped. The real install, setup, update and repair entrypoints under tests/studio/test_path_probe_access_denied.ps1: all checks passed. * Rewrite the Add-Type fallback suite for a design with no compiler in it Staging CI caught a whole test file I never ran: tests/python/test_windows_installer_addtype_fallback.py, 1612 lines built entirely around the compile-and-retry path this branch removes. Five of its tests failed. My local runs covered tests/studio, the Windows concurrency guard and a handful of tests/test_installer_*, and none of them reach this file. Two of the five were collateral. LOCK_CHAIN extracts the resolver chain function by function, and the chain gained Test-StudioCanDefineNativeTypes, New-StudioEmittedNativeType and Get-StudioNativeFinalPath, so the link-resolution tests were calling helpers the harness never defined and reading the CommandNotFound as "could not resolve". The other three described the removed design and had to say something else: - test_install_lock_is_acquired_when_the_compiler_fails is now test_install_lock_is_acquired_when_the_native_helper_is_unavailable. The bug in #9140 was a throwing Add-Type; the same bug now is a host that cannot define a type at all, which is the same thing to every caller, and NO_NATIVE overrides the gate both Constrained Language Mode and Dynamic Code Security come through. - test_the_compiler_is_not_retried_for_every_path asserted CALLS:2, one attempt plus one private-TEMP retry. It is now test_the_compiler_is_never_called_however_many_paths_are_resolved and asserts zero. The caching that count stood in for gets its own test against the emit. - test_the_private_temp_retry_recovers_the_native_helper becomes test_the_native_helper_is_built_with_no_usable_temp_at_all. The retry existed because compiling wrote a source file into %TEMP%; emit writes nothing anywhere, so the same unusable %TEMP% is simply irrelevant, and the assertions that TMP and TEMP are handed back untouched carry over unchanged. New: test_a_dead_compiler_is_not_something_the_installer_can_notice. Sabotaging Add-Type has to change nothing at all now, which is the property worth pinning, and a failure there means a compile came back. Renamed to test_windows_installer_native_type_fallback.py, since the fallback it guards is no longer about Add-Type. Nothing references the old name. The file: 58 passed. Also checked, and NOT mine. Five other failures in that staging run reproduce on a clean worktree of origin/main at b75ed88156: tests/test_windows_amd_gpu_scan_fallback.py 4 failures tests/test_deliberate_crashes_suppress_cores.py 1 failure test_installer_restores_the_private_handoff_after_setup asserts $previousRocmGfxHandoff appears after the --with-llama-cpp-dir error, and on main it appears before it. PR 10494's branch passes it because it predates whichever commit reordered them. Left alone here rather than fixed in a branch about something else. Same for the two tests/python failures outside this file (test_e2e_no_torch_sandbox, test_rl_config_pickling): both fail on origin/main and neither reads install.ps1. The Clean machine install leg fails on linux ubuntu2404-arm-root with "built from source: nvidia-npp-cu13", an aarch64 wheel that does not exist. A Windows PowerShell change cannot reach it. Wider run on this branch: tests/python 2322 passed, 129 skipped, 2 pre-existing failures; tests root filtered to installer and Windows, 1503 passed, 35 skipped, 4 pre-existing failures. * Stop promising an install is unaffected when a scanner is on the process tree The degraded warning said "Continuing with the PowerShell resolver; installation is unaffected." That is a promise this line cannot keep. Every condition that stops a type being defined here is security software or a lockdown policy acting on the process, and the same thing can be acting on the rest of the run; saying the install is fine is exactly the reassurance a user then finds was wrong. It also said nothing useful. The narrower claim is both true and actionable: the installer continues, and the answer it continues with is inexact in a specific way, so a path is compared as written rather than as stored. * Fail CI when the installer actually spawns a C# compiler The text guard in test_installer_av_shapes catches an Add-Type that is written in one of these scripts. It cannot see a compile reached through a module, a dot-sourced file or a generated here-string, and it cannot see one a dependency performs while our own process tree is what a scanner scores. Bitdefender scored the chain, not the bytes: unsloth-studio.exe -> powershell.exe -> csc.exe -> %TEMP%\<random>.dll so there has to be a lane that runs the installer and asserts on processes. Two independent detectors, since either alone can be defeated by something that is not the code under test. Security event 4688 is authoritative about what ran and sees a compiler spawned at any depth. A sweep of every temporary directory for new .dll, .cmdline and .rsp files survives auditing being silently overridden by a machine policy, and catches an assembly that was written and cleaned up. The positive control runs first and is not optional. A detector that sees nothing reads exactly like a clean run, and every part of this can fail to apply without saying so. The job compiles one throwaway type with Add-Type and requires both signals to fire; if either does not, it fails as a broken detector rather than going on to report a meaningless pass. The installer is spawned as a child powershell.exe rather than dot-sourced, because the shape under test includes what the installer's own children do. What this cannot answer: a hosted runner has neither App Control nor Smart App Control enforcing, so a green run says the install path spawns no compiler and drops no temporary DLL. It says nothing about what an enforcing machine would do. Also corrects a stale comment: setup.ps1 emits its console thunk now too, so neither script compiles anything. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Define the dynamic assembly by both spellings, and run the resolver test in CI Review raised that Windows PowerShell 5.1 on .NET Framework does not expose the static AssemblyBuilder::DefineDynamicAssembly, and that the emit would therefore throw into the catch, cache the resolver as unavailable, and leave every desktop install on the lexical fallback, which can give a long path and its 8.3 alias different install mutex names. The premise looks wrong: the API reference lists netframework-4.5 through netframework-4.8.1 in the applies-to range for both overloads, alongside the .NET Core monikers, with mscorlib.dll among the assemblies. But nothing in this repository can run a .NET Framework host, the claimed failure is silent rather than loud, and the cost of covering both is four lines, so New-StudioDynamicAssembly tries the static and falls back to AppDomain.CurrentDomain.DefineDynamicAssembly. Order matters and is not arbitrary. Measured on pwsh here: the static returns a RuntimeAssemblyBuilder with an empty Location, and the AppDomain spelling is absent and throws RuntimeException. So the AppDomain call cannot be the first try without breaking the host we can actually test, and it is the fallback instead. Registering tests/studio/test_native_path_resolver_degrades.ps1 with the CI step was also raised, and it earned its place immediately: adding the helper broke 18 of its checks, because it loads the resolver's functions by name and the new one was not in the list. That is exactly the regression it exists to catch, and until now nothing ran it. It is now in the PowerShell installer unit tests step beside the others, and two source checks assert that neither spelling can be dropped. * Address review: fix the watcher, measure the desktop mode, and extract the new helper Seven items, all correct, three of them bugs in code added yesterday. Each was reproduced before it was fixed. The watcher returned the wrong thing. `& $Action` leaves the action's success stream on the function's output, and the installer action tees its log, so every line was emitted ahead of the result hashtable: the caller's $seen became an Object[] of length 2 and $seen.Compilers failed under Set-StrictMode against a string rather than reporting the measurement. Piped to Out-Host now. The watcher also died on a clean machine. Get-StudioTempArtifacts returned an empty array, PowerShell unrolled it to nothing on return, and the two-argument HashSet constructor rejects null, so Invoke-WithCompilerWatch threw a MethodInvocationException before running even the positive control. Both reproduced in miniature on pwsh; both now return a Hashtable with the action still logging. The 4688 query had no end time, so a compiler started by anything else while the recursive temp sweep was still running counted against an action that had already finished. The window closes before the sweep now. The prose claimed a process tree, which the implementation never did: 4688 carries the creator's pid, but a compile can be several processes deep and those pids have exited by the time the log is read, so the ancestry is not reconstructable after the fact. The docstring says that instead of implying otherwise. The job measured only the shell mode. install.rs initialises its argument list with --tauri unconditionally, so a compile reached through a Tauri-only branch was invisible to a job triggered by changes to that launcher. Both modes run now; the desktop leg unpins UNSLOTH_STUDIO_HOME rather than changing the mode, since the desktop mode refuses it, and it is not gated on exit 0 because the question there is only whether a compiler ran. The path filter omitted the watcher, which implements both detectors, so a PR touching only it could disable the detection without running the control. New-StudioDynamicAssembly was missing from the concurrency guard's three extraction lists, and that test caught it exactly as designed: "_mutex_helpers extracts functions that call ['New-StudioDynamicAssembly'], which the harness never defines". Two tests were failing and I had not run that file. The resolver test counted every matching temp file touched in the last minute as this emission's output. Demonstrated: one unrelated .dll dropped in the shared temp directory makes the old rule report TEMP_ARTIFACTS: 1 and fail a run that wrote nothing. It is a set difference against a snapshot taken before initialization now, which is what the assertion actually means. * Fix three CI failures this PR introduced The compiler check itself was broken, which is the worst of the three: it failed on every healthy runner and would have failed on a machine that really did spawn csc.exe for the same reason, so it graded nothing. auditpol returns an ARRAY of lines, and -notmatch on an array is a filter, not a test: it returns the elements that do not match. On a runner where auditing turned on correctly it still returned the three header lines, and a non-empty array is truthy, so the guard threw with "process creation auditing did not turn on" directly under output reading "Process Creation Success". Joined to one string and matched on the subcategory line. Checked against the two other match operators in the PR; both operate on single strings and are fine. tests/studio/test_installer_av_shapes.py reads a workflow file, and the repo requires any such guard to run in workflow-trigger-lint, the one job with no paths filter, so a PR that edits only workflows still runs it. Added there. The VT parity test sliced Enable-StudioVirtualTerminal without the helpers it calls. Extracting the emitted-type path gave that function two new callees, so the reconstruction representing the predecessor died with an unrecognised command and exited 1. That reads as "the predecessor disagreed" when in truth it never ran. The slice now carries Test-StudioCanDefineNativeTypes, New-StudioDynamicAssembly and New-StudioEmittedNativeType, and both reconstructions parse clean with no unresolved commands. The assertion also printed only the passing run's stderr, so the reconstruction's own error was never shown and the failure was undiagnosable from the CI log. It prints both sides now. * Studio: fix three Windows-only installer tests that the cross-OS run caught Only reachable on a real Windows or macOS runner, so none of them had run before. test_versioned_native_helper_loads_after_older_installer_type asserted on UnslothStudioFinalPathV2 and called GetProcessImagePath on it. The type is UnslothStudioFinalPathV3 and that method belongs to UnslothStudioProcessImageV1, so under ErrorActionPreference Stop the script threw and the test reported a bare exit status 1. It now checks that the versioned type is defined and that the resolver returns a real path even though an older installer already put UnslothStudioFinalPath in the session. _run_powershell no longer passes check = True. The PowerShell error was being discarded, which is what made the failure above unreadable; the output is now attached to the assertion. test_the_sweep_only_takes_directories_the_allocator_could_have_made created ust-<pid>-abcdef01 and ust-<pid>-ABCDEF01. NTFS and the default macOS filesystem are case-insensitive, so the second mkdir raised FileExistsError before the sweep ran. The uppercase directory now uses a second dead PID. test_unequal_paths_are_unknown_rather_than_different_when_inexact reached the inexact path by making Add-Type throw. The installer no longer calls Add-Type, so on Windows the emit succeeded, the answer was exact, and two directories compared false. It now goes through the capability gate. * Studio: refuse reflection emit under any user-mode code integrity policy Microsoft documents that App Control option 19, Dynamic Code Security, always blocks loading unsigned assemblies built with System.Reflection.Emit, that there is no audit mode for it on Windows 10 or on Windows 11 before 24H2 (it is turned on and enforced there even when the policy is in audit mode), and that a blocked dynamic load usually stops or crashes the parent process. The gate only refused enforcement status 2, so a machine running an audit-mode policy with that option would have reached the emit. Win32_DeviceGuard does not report the option bit, and no catch recovers a stopped process, so any active user-mode policy now takes the lexical path instead. That degradation is one the installer has always had and completes on; a crashed installer is not. Also from the same audit: - The open now sits inside the region that closes it, in both native helpers. A SafeFileHandle had a finalizer as a backstop between the open and the using block and an IntPtr has none. The handle is opened with desired access 0 and full sharing, so a leaked one blocks nothing, but the window did not need to exist. The close is guarded, since the failure paths now reach the finally. - CreateType can publish a type and then throw on the way back. The compiled version checked for that before giving up and the replacement did not, which turned a recoverable failure into lexical resolution for the rest of the run. - CharSet is declared per import rather than Unicode for all of them. It selects which export the runtime probes first, and CloseHandle, OpenProcess and the three console calls were CharSet.None in the C# this replaces. Every name here exists exactly as written so both orders resolved, but the metadata now says what the declaration said. Tests cover the gate on both scripts at status 0, 1 and 2 and with no provider at all, the charset carried by each emitted import, and the published-then-threw path. * Studio: run the degradation test on the host it describes, and make the stub real Two defects the cross-OS audit found, neither of which any lane could have reported. tests/studio/test_native_path_resolver_degrades.ps1 ran only on a windows-latest job. Its premise is a host where the emit succeeds and kernel32 does not resolve: it asserts the native rung answers null and the ladder reports the result inexact. On a healthy Windows runner those calls succeed and those assertions are false. It moves to the Linux leg of cross-platform-parity-ci.yml, and the file now exits early on Windows with the reason rather than failing a dozen checks that describe a different machine. The throwing-resolver test still defined UnslothStudioFinalPathV2, which nothing calls now, so the resolver fell back because the type was ABSENT and the test passed without ever reaching the stub. It defines V3, which is what the resolver calls, and counts its invocations: the assertion is now that the throwing path ran four times, not merely that the outcome looked the same. * Studio CI: void the desktop-mode measurement when it never ran, and close the watcher window after the baseline Two review items, both correct. The desktop-mode leg accepted any exit code, which is right: a --tauri install can decline for reasons that have nothing to do with whether a compiler ran. It also accepted a run that stopped at argument validation, which spawns nothing, so both detectors came back empty and the step printed "no compiler" having measured nothing. It now requires the log to show the run reached install work, and calls the result void rather than clean when it did not. Watch-ForCompiler recorded $since before the baseline temp sweep. That sweep walks every temp root recursively and can take seconds, so an unrelated csc.exe started during it landed inside the window and failed either the positive control or the installer measurement for something neither did. The baseline is taken first and the window opens after it, which is the same ordering the closing side already used. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: ask a child process whether emit works, instead of guessing from the policy status The gate refused every machine with an active user-mode code integrity policy, audit or enforced. That is safe but expensive: option 19 Dynamic Code Security is what blocks unsigned System.Reflection.Emit assemblies, an audit policy without it emits perfectly well, and Win32_DeviceGuard does not report the option bit. Refusing everything sends every audit-mode machine down the lexical path, where identity is inexact and the install lock, the venv-in-use scan and the --tauri root comparison are all weaker. Allowing everything risks the process, since Microsoft documents a blocked dynamic load as usually stopping or crashing its parent. Neither guess is necessary. When a policy is active, a child PowerShell attempts the same emit and reports back. A block stops the CHILD, which is the entire reason for asking there rather than here, and silence is refusal, so a probe that cannot be spawned lands on the lexical path rather than on optimism. A machine with no policy, which is nearly all of them, answers as before and pays for no extra process. Tests cover both scripts: no policy answers without spawning anything, an active policy of either kind takes its answer from the probe and calls it exactly once, and the real probe returns a boolean on this host without throwing. * Studio: keep the emit probe free of double quotes so 5.1 receives it intact The probe I just added passed its body to the child with -Command. Windows PowerShell 5.1 binds a native command's arguments the legacy way: it wraps the value in double quotes and appends the body verbatim, without escaping the double quotes inside it. The first one inside closes the wrapper, the remainder is re-split on whitespace, and the child ends up running if (UnslothStudioEmitProbe -as [type]) which is a command lookup that throws into the probe's own catch. The probe would have answered no emit here on every 5.1 host, which is the interpreter studio/src-tauri/src/install.rs spawns, so the gate would have refused exactly the machines it was rewritten to rescue. Reproduced by putting pwsh into that binder with PSNativeCommandArgumentPassing: the same probe answers False under Legacy and True under Standard. The body uses single quotes throughout, which leaves nothing for the binder to lose. Passing it base64-encoded also fixes it and is what the documentation suggests, but base64 PowerShell is the shape this whole change exists to stop resembling, and tests/studio/test_installer_av_shapes.py rejects it outright. Two tests, because one of them would not have caught this on its own: the probe runs for real under both binders and must answer True under each, and the body is asserted to contain no double quote, which is the invariant a future edit trips without any sign pointing back at the quote. The host lookup also accepts the extension-less leaf now, so the function is one a non-Windows lane executes end to end rather than a Windows-only path that nothing runs. * Studio CI: pin the two actions in the no-compiler workflow to SHAs The rest of the repository pins actions/checkout and actions/upload-artifact to commit SHAs, 83 uses to 5 and 59 to 3. This workflow used the mutable v4 tags, and a tag can be retargeted, so a security-sensitive job would have run whatever the tag later named. Same SHAs the other workflows use. * Studio CI: trigger the parity lane when the degradation test itself changes Moving tests/studio/test_native_path_resolver_degrades.ps1 to this lane left it in no path filter. The Windows smoke workflow it came from is still triggered by tests/studio/*.ps1 but now deliberately skips the file, so a PR editing only that test would have run nothing that executes it. Added to both filter blocks. * Studio: declare the console mode parameter Out, as the C# did GetConsoleMode's second parameter was declared "out uint". DefinePInvokeMethod has no argument for parameter direction, so a by-ref type alone emits "ref", which leaves both In and Out unset. Reflected over the emitted method, IsOut was False where the compiled declaration gave True. Nothing observable depended on it: the value is blittable and both call sites initialise it before the call, so the marshaller pins the address and the writeback happens either way. It is one DefineParameter call to make the metadata say what the declaration said, and metadata is what a reader and any future marshalling change go by. The emitter now honours an Out key listing parameter positions, and the test asserts both halves: that the emitter produces IsOut True when given one, and that the real GetConsoleMode declaration still carries it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the emit probe answer only when it is sure, and bound how long it waits Four defects in the probe, all of them mine and all reachable. Unknown policy state was treated as no policy. The CIM query has a catch, and a throw, a missing object or a missing property all fell through to "allow". The case that lets through is option 19 enforced on a host whose query happens to fail, where the documented outcome is a stopped process. Read-and-zero is now the only outcome that skips the probe; anything else asks. A marker followed by a bad exit was accepted. The whole test was whether the output contained the marker, so a child that printed it and then crashed answered yes, and so did a line that merely contained the marker as a substring. It now requires exit code zero and a line equal to the record. The child can run in ConstrainedLanguage where its parent, an approved script, runs in FullLanguage. That child has measured a different machine, so the record carries the mode and only FullLanguage counts. There was no deadline. The call operator waits forever, and forever is reachable: a security product inspecting a freshly spawned interpreter, a wedged runtime start, a child that blocks on shutdown. It runs through a Process object with a twenty second wait and a kill, and both streams are redirected and drained. That last part is not only about deadlock: an inherited stderr lets the probe write into the installer's own error stream, which the desktop app reads, and anything the child spawns holds that handle open. A test that left a grandchild behind hung for two minutes on exactly this and now finishes in twenty seconds. Also here, because the growth of setup.ps1 exposed the gap: tests/test_installer_ profile_hardening.py parsed install.ps1 only. Every other test reads these files by extracting one function, and an extraction still parses when the file around it does not, so a broken brace at file scope passed the whole suite. It now parses both entrypoints, and it fails on the damage it exists for. * Studio: score the image that ran, and make the optional Out key optional again Four corrections from a fresh audit of this branch. The emitter read $import.Out unconditionally. Almost no import declares one, and reading a hashtable key that is not there is fatal under a caller's Set-StrictMode -Version Latest, which raises PropertyNotFound rather than returning null. install.ps1 sets strict mode Off for itself; studio/setup.ps1 sets none at all, so whatever the caller has is what runs. It now asks ContainsKey first, the way it already did for the CharSet key, and a new test builds a type from an import with neither key under the strictest setting. The compiler watcher searched the whole rendered 4688 message, which contains the command line, so cmd.exe /c echo csc.exe scored as a compile. It now reads NewProcessName out of the event XML by name and matches the leaf whole. The classification moved into its own function so it can be exercised without a Security log, and four cases cover it: a real compile, a command line mentioning one, a path argument and a similarly named image. The watcher also turned every Get-WinEvent failure into an empty result, so a Security log it could not read was indistinguishable from a clean run and the job printed "no compiler" having seen nothing. The two are now separated structurally rather than by matching localised message text: ask the log for any one record. If that succeeds the filter genuinely matched nothing; if it fails too, the run is void and throws. The one second the window reaches back into the baseline sweep is now stated instead of claimed away. Two test harnesses extracted Test-StudioCanDefineNativeTypes without the child probe it now calls, so with a Device Guard status of 1 or 2 they died with CommandNotFound rather than measuring anything. Both extraction lists carry it, and the dependency-completeness test now covers _final_path_helpers, which is where the omission lived. The no-compiler lane's desktop leg now passes --local. Without it the installer installs the released unsloth and delegates setup to the copy inside that release, so a PR touching only studio/setup.ps1 triggered this workflow and then measured a different revision of the file it changed. The leg refuses to report a result unless the overlay is in the log, and records the measured path and hash. * Studio: tell a blocked machine apart from a probe that never answered The gate had two outcomes and needed three. A child that ran and refused is a policy; a child that could not be spawned, was killed at the deadline, or lost its output is a process that failed. Both cached false for the whole run, and the consequences are not small: refusal degrades path resolution, two unequal roots then compare as unknown rather than different, the installer takes both the SID lock and the path lock, and an unrelated install elsewhere on the machine turns into "The managed Unsloth environment is busy". The compiled version this replaces tried twice before caching a negative. So the probe now records why it answered as it did, and the gate retries only the case where nothing was established. A machine that is genuinely blocked still pays for exactly one probe. Six cases cover the classification and two cover the retry rule, in both entrypoints. The degradation warning said "this host enforces user-mode code integrity" for all of it, which names a machine setting that whoever reads that log then goes looking for and does not find. It now says that only when a child ran and refused. The process-image helper asked the gate again even in a session that had already emitted the path type. The compiled version could not disagree with itself there, because one type carried both helpers; two types can, and the way they do is a session that emitted one, was interrupted, and came back to a probe that now fails, keeping native path resolution while silently losing process inspection. A type already published in this process is stronger evidence than any child, and is what settles it now. * Studio: a type this session already emitted outranks any probe Three places asked the gate before looking for the type they wanted, so a single failed probe discarded a helper already loaded and working in the same process. The compiled version checked the type first, and there is nothing a child process can tell us about emit that a published type has not already settled here. The console helper, the shell-icon refresh and the process-image initializer all check the type first now. The probe itself did not apply the rule it exists to measure. CreateType can publish a type and then throw on the way back, which the parent recovers from; the child had that check inside its try, so the same success answered no. It is outside the try now, on one line with no closing brace in column 0, because the body sits in a here-string that starts at column 0 in both entrypoints and the extraction the tests use ends a function at the first line that is exactly its closing brace. A block there silently truncated sixteen tests. * Studio: bound the policy query, and say what that bound does not cover The Device Guard query was synchronous and unbounded. It is bounded now, and the comment states what the parameter actually does: OperationTimeoutSec limits the CIM operation on a responsive target, does not interrupt DCOM connection setup, and does not override a wedged provider, where the server's own timeout wins. It is worth having for the slow case and it is not a hang guard, so it does not claim to be one. The installer already queries CIM for adapters and processes, so this is not a new exposure; the child probe is the part that carries a real deadline. * [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>	2026-09-09 05:53:47 -07:00
Daniel Han	971eb36f48	Trim comments in tests, scripts, CI helpers and unsloth_cli (#10113) ... * Trim comments in tests, scripts and CI helpers Comment-only pass over 453 changed files under tests/ (excluding the studiobench harness), scripts/ and .github/scripts/. 17,749 -> 10,635 comment lines (-40.1%) 1,183,079 -> 757,190 comment characters (-36.0%) Distinctive-fact retention against the base commit: 93.2%. A sample of 100 restoration and deletion decisions was judged by hand: 98 correct. No comment line exceeds 120 physical characters. Nothing outside comments changes: no code, docstrings, string literals, licence headers or pragmas. Verified with an AST code-only signature of every file against the base. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Put the moved comments back on the code they describe The signal-handler rationale in launch.py had been reattached to the handler registration loop, which has no try/finally, and its except-branch note had been split into a fragment starting mid-sentence with the noqa marker duplicated into the prose. The PORT_FILE launcher comments had been reattached to the _studio_root_id caching test, which does not touch PORT_FILE. * Put the section banners back, and use plain dashes in comments The trim removed the rule-line comments that index these files. Tests locate a region by finding its banner and end it on the next one, so a missing banner is a broken anchor rather than lost decoration. Each is restored above the code line it preceded at 1d2679f; where that anchor is not unique the banner is left out rather than guessed at. Em and en dashes are rewritten as plain dashes inside comment tokens only. String literals are code and are untouched, so the diff stays comment-only. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Put reattached comments back on the code they describe The comment pass moved a number of explanatory comments off the code they were written for and onto unrelated statements nearby, so they now read as false or as truncated fragments. This restores each one at its correct site: - lint_duplicate_definitions: the import-key rationale goes back on the emitted Finding key instead of being half-pasted onto `implicit =`. - lockfile_supply_chain_audit: "Empty by default" belongs on the empty CARGO_IOC_STRINGS tuple, not on the non-empty source allowlist; the missing-lockfile and traceback notes go back on their own branches. - scan_packages: "archives landed in dest" describes the successful pip download, not `if not downloaded:`; the backslash-continuation note goes back on the continuation check in _scan_line_end. - interrupted_install_probe: the start_new_session/getpgid note belongs on the posix branch that calls getpgid, not on the win32 branch. - virustotal_scan: the missing-secret rationale goes back on the skip path and the :⚠️: note back on the detection print. - scan_npm_packages: the rmtree cleanup note goes back on the rmtree. - check_frontend_dep_removal: one per-key rationale per key again, instead of five concatenated onto browserslist. - profile_startup: failed-launch and nothing-measured notes swapped back. - enforce_kwargs_spacing: fold-rejection reasons back on the guards that reject for them, and the f-string fold comment back in order. - tests/conftest: the bitsandbytes broken-wheel note back on the bitsandbytes handler. Comments only; no code, string literal, docstring or pragma changes. * Repair the banner restore: drop duplicate headings, keep the blank line The scripted restore re-added each section banner without removing the de-bannered plain heading the trim had left behind, so a heading appeared twice, once ruled and once bare. It also inserted the banner flush against the next line of code where the base had a blank line after it, which turns a section separator into a doc comment for whichever def follows. Both are the same fault the trim itself was being screened for, introduced by the remediation rather than by the trim. * Reattach 12 review comments to the code they describe Each of these was a rationale that ended up on a neighbouring test or assertion, or a fragment cut mid-sentence, so it described something other than the line under it. - test_gguf_disk_preflight: the 3GB-embeddings arithmetic belongs to the untied case, not the tied one (2GB there). - lockfile_supply_chain_audit: reassemble the workflow-command escaping rationale on the skip branch. - studio_smoke/multi_turn_chat: restore the retry-policy explanation in main(). - test_virustotal_scan: restore the advisory-posture rationale on the scan step. - test_amd_fastpath_probe: return each ROCm rationale to its parameter row. - test_cuda_repair: the Blackwell span note goes back on the Blackwell test; complete the Kepler sentence. - test_windows_rocm_bnb_version: realign the sitecustomize / opt-out rationales with their cases. - test_dataset_num_proc: the Pool(1) note belongs to the low-memory auto path. - test_install_whisper_prebuilt_logic: decoy-DLL, libomp, build-bump and same-ggml notes back on their own tests; the link copy fallback needs none. - playwright_loaded_models_indicator: stale-eject notes back on their sections and the preference banner restored. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Restore the review comments that were cut mid-sentence or reattached to the wrong code * Restore the comment tails the 120-column pass cut off The trim enforced the column limit by chopping the physical line rather than rewrapping it, so a block's last sentence stopped mid-thought and the clause carrying the reason was gone: -# ...fails in a second rather than +# ...fails in a second rather than after the quantise and the multi-gigabyte save. -# fp8 / mxfp8 need bf16 weights, so skip non-bf16 Linears; +# fp8 / mxfp8 need bf16 weights, so skip non-bf16 Linears; nvfp4 handles fp32. Mirrors the runtime gate. 883 blocks across 266 files, restored from the merge base and rewrapped at word boundaries within 120 columns. Blocks that end on proper punctuation followed by a new sentence are left alone: those were shortened on purpose, and only a base that continues the same sentence where the head stops counts as a cut. No word is invented; every restored word comes from the base. Pragmas, shebangs, coding lines, licence text and rule-line banners are skipped. Dangling truncations drop from 336 to 77. Full suite is 65 failed / 18683 passed, matching main exactly, so no test changes behaviour. * Restore comment tails cut inside a parenthetical or abbreviation The first repair pass keyed on terminal punctuation, so a block cut after "(e.g." or mid-code-span looked finished and was left alone. Balance the brackets and backticks instead, and treat a trailing abbreviation as mid-sentence: -# ...skip the whole module up front on runners without it (e.g. +# ...skip the whole module up front on runners without it (e.g. CPU-only) before importing unsloth, otherwise +# collection errors instead of producing a skip. 22 further blocks across 20 files. _ts_deps.py is fixed by hand: it had also lost a sentence from the MIDDLE of the block, so it was not a prefix of the base text and no prefix rule could reach it. * Reassemble the review comments that were left scrambled or cut short Restore the full rationale for the comments the trim pass left as reversed or truncated fragments, and put the openenv, reasoning-effort and recovery window notes back on the code they describe. * Put the scrambled comment blocks back over the code they describe * Put the inline labels back on the entries they label * Restore an inline label lost in the merge * Restore the facts the reworded comments dropped * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Move the displaced notes back onto the tests they describe * Re-pair the fixture rationales with their fixtures * Put the relocated notes back and finish the sentences the trim cut * Restore the dropped facts and correct the ones that were never true * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Unfuse the merged notes and drop the reformatting the trim smuggled in * Restore the clauses the trim cut off and unfuse the notes it merged Thirty sites where a comment was shortened past the point it still said anything. Most ended on a colon or a dangling word, so the reason the branch exists was gone while the sentence introducing it stayed. The rest had absorbed a neighbouring test's note, which left one row documented as another row's behaviour. Each restored clause was checked against the code under it rather than copied back on trust. Three were not restored: a pointer to a measurement that is not in the function it names, a claim that a missing import fails as NameError when it fails at collection, and a redundant restatement in the autoscroll check. tests/kaggle/t4_smoke/training_evidence.py ends up identical to main, which is the right outcome for a file whose only trimmed sentence was load-bearing. * Give back the halves of the comments the trim cut off Two hundred and twenty comments across a hundred and twenty files had been shortened to the sentence that introduces a fact and none of the fact. A note ending on a colon or on "so" or "carried over" still reads as a comment and still parses, which is why every structural check has been passing over them: a truncated comment is a well-formed comment. Most of these were only findable by asking what a block ENDS on rather than what it starts with. The seven passes before this one all keyed on the first line, so they were blind to the commonest damage in the whole trim. The supply-chain audit had seven "Mini Shai-Hulud May-12 wave:" headers with the package names deleted, which was the entire content of each one. A handful were worse than truncated. A "can't fit on one line, so leave it" note had moved from the test asserting changed is False onto the one asserting changed is True; the sm_61 and sm_70 rationales in the CUDA repair tests were stacked on a test that exercises neither, leaving their own tests bare; an unpinned-latest note sat on a test that stubs a working install on disk. Restored clauses were checked against the code under them, not copied back on trust. Around a dozen were left out or rewritten because the base was wrong about the code as it stands: a pip stub credited with an append it does not do, five exports said to be filled by a loop that fills one, a __dict__ rationale that argues for the opposite of what the loop needs, and UI strings quoted as current that the sources no longer use. * Put the fused row notes back on the rows they are true of Ten sites where a comment described a sibling. The torchao skew table had three rows' notes concatenated onto the 0.18.0+cu130 row; the revision-forwarding table had the adapter-tokenizer exception one row below the only row it holds for; the vision LoRA pair had the tied and untied cases on a single anchor. Four of these were mine, not the trim's. The 120-column rewrap left an orphaned continuation line sitting on a class, one repair copied an untagged-build note onto a fixture that pins a URL and has no untagged build in it, another duplicated a namespace-package note and pushed the interrupted-install label onto the wrong row, and a fourth moved a collision note onto a fixture with a single host, where it is false. A pass that rewrites 220 comments will get some of them wrong, and these are the ones it got wrong. Two reported sites were left alone. The masking note in the VirusTotal client already carries its full clause, reflowed; the IOC scan comment reads complete, and its missing "4." went with the numbering on every other step in that function. * Move the row and branch notes back onto what they describe Nineteen sites. The token-count parity table had every row comment one param too low, so each seeded reasoning style was documented as the next one; the font-scale contract had two rationales slid down a continue each; the validator fixture banner for R-INST-001 sat under R-INST-005, leaving three tests unlabelled and two double-labelled. Three of these I caused. The parity table was misplaced by the previous commit, which moved fused notes onto rows without checking the seeds underneath them; a timing bound landed in a test that has no timing assertion; a restore note was reinserted between the two commands it claims to cover instead of above them. The pacing benchmark had lost its measurement table entirely, including the #7892 result that is the reason there are two budgets rather than one headline number. Restored with the columns intact. * Undo the damage the last four repair passes did Twenty-five sites, and almost none of them were the trim's. Round nine was called "move the row and branch notes back onto what they describe", and it moved a good many onto things they do not describe: the xpu swap tests had the pinned-mirror rationale on the test that asserts the index is dropped, the cuda repair file had a whole run of rationales slid one test upward, and several notes were duplicated onto a neighbour rather than moved. Where a clause was already correct on its real anchor, the copy is deleted rather than parked somewhere new. Separately, this PR had been changing code, and the gate could not see it. Five files: three where a repair split a tuple across lines to make room for a restored trailing comment, two where the trim deleted a trailing comment and then joined up the wrapping that existed only to carry it. Parentheses and line breaks leave the parse tree untouched, so an AST comparison calls all of it comments. Every one is back to base byte for byte, with the comments that justified the wrapping restored. scripts/code_reflow.py is the check for it: same tokens in a different arrangement is a code change, whatever the AST says. * Let ruff rejoin the two lines the shorter comments freed Both were split only to make room for a trailing comment. Round ten cut those comments back to what is true of the row, the expressions now fit, and the formatter joins them. Running it is not optional: pre-commit.ci was red on exactly these two. * Delete the stray copies rather than move them again Five sites. Three were duplicates: the correct sentence still sits on the test it describes, and a repair pass left a second copy on a neighbour. Moving those would have been the fourth round of shuffling one clause between two tests, so the copies are simply gone. The mislabelling was the point in each case. An empty handover tag and an absent one are different precedence rules, and the PowerShell 7.5 parsing note had been copied from the empty-string test onto the one whose variable is unset. A non-zero-exit rationale sat on a parser test that asserts a successful exit. The other two are the trim's own: "#5854)," with its subject cut away, and a regex description that stopped before naming the self. prefix it matches, which is what explains the narrower filter below it. * Put the nine slid notes back on what they assert Seven had moved one branch or one call down. The autocast one had landed on the case that forces float32 and asserts dtype float16, directly under a note saying there is no dtype at all; the JAX rationale sat on the USE_FLAX env-var case, whose module map has no jax in it; the kill-and-sentinel note sat on a test that only checks a GPU row regex. Each is back on the assert it describes, with a tenth found in the same file as the probe timeouts. Two are the trim's own. A mid-block deletion in the settings-tabs driver spliced two half sentences into "blocking a / error-handling regression and was a stale constant"; the macOS slot table lost the only statement of why macos-14 is absent from a set named for images GitHub still schedules. The retirement dates are restored, verified against all three workflows that carry them. The settings-tabs note is shorter than base on purpose. It now says the invariant, that blocking a panel must not collapse the dialog, and drops the tab count, which nothing asserts. * Move twelve more notes back onto the branch they are about All twelve had slid onto an adjacent test or branch, and in each the displaced note contradicted what it now sat on. The CPU-backend note about a wheel being left alone had landed on the test asserting a repair runs; the XPU one about a dispatched shard sat on the single-device case; the redirect-body note sat in a test that never binds served; the draft-state refusal sat on the test where the target is present and the run returns 0. One lost text on the way as well: the missing-target rationale had been cut to its first two sentences, dropping the reason the draft and prerelease refusal above it would otherwise degrade into no check. Also a rewrap that duplicated its own tail instead of replacing it, so the merge-budget note read "else a sharded own device, not GPU0, else a sharded model OOMs". * Return six notes to the test whose behaviour they describe Eight reports, six defects: two pairs described the same misplacement from either side. One was worse than misplaced. "A comment naming a probe is not a probe" had moved onto the metadata scan, whose predicate matches the raw line with no comment handling at all, so it claimed an exemption that scan does not implement. It belongs on the Test-Path scan, the only one that splits on # before matching. Moved, not reworded. The rest argued against the test they sat on. The lazy-trl note was on a stub with no __getattr__ to raise; the bogus-variant note was above a request sending a valid variant; the imported-TensorFlow note was on the case that supplies no tensorflow and asserts the flag is cleared; the 62-token rationale was on the test that asserts 12. * Move ten more notes onto the statement they explain Seven had slid off their anchor, and in most the stray was also cut short, so the fix was to restore the whole sentence where it is true and delete the fragment. The torchao dependency reason sat on a test about re-verification rather than on the --no-deps assertion it justifies; the bitsandbytes heading sat on a test that fetches quantizers_utils.py; the no-selection latch note sat on the delete block, which asserts nothing reaches the action. Two are the trim's own truncations. The stand-aside rationale in the shortcut driver stopped at a colon, losing why Enter on a focused Deny button would otherwise be defaulted away into approve. The kwargs-union case stopped at a semicolon, and the trim had eaten the leading ** from **kwargs as well. One was about the driver rather than a test: the JSON note sat beside the final print, where it described the document, instead of on the stderr redirection that is what actually keeps stdout clean. * Restore what the reflow cut, including three markers it mangled Nine sites. Most are the trim's own truncations: the dill fixture had lost the property that makes lookup by reference fail, so an otherwise unexplained __module__ = "builtins" sat there with no reason; the rope drift test had two clauses spliced into "every buffer keeps scaling in a buffer"; the playwright guard had lost the three command shapes it matches and why the regex is not anchored on an engine list. Three are a shape I had not seen. The reflow rewrote the `#:` variable documentation markers in the fast-copy constructs to `# :`, and dropped stray colons into the wrapped prose, leaving "text : node". Those markers are syntax, not punctuation to be tidied. One was misplacement rather than truncation. The dotted-import rationale had moved onto the second copy of its case and lost its tail, leaving a dangling "and" above a capitalised "Still caught" and the first row bare. Restored where base has it, stray deleted. lint_duplicate_definitions self-test still passes, 63 cases. * Put five drifted comments back on the code they describe The trim slid several notes onto neighbouring tests, where they describe something they no longer sit on: - test_qat.py: the TODO moved off _test_full_model_fake_quantize, whose leading underscore is what keeps pytest from collecting it, onto a test that is collected and was never disabled. - playwright_chat_ime_i18n.py: 6f carried a stray copy of 6e's compositionstart note above an Enter KeyboardEvent, and had lost its own rationale for why Enter reports isComposing=false while composingRef is pinned. - test_install_node_prebuilt_logic.py: the short-circuit note landed on the transient-download-failure test, leaving the SHA-enforcement test bare and dropping the carve-out that a different usable version is still kept for offline resilience. - test_chat_autoload_failure_gate.py: the default/namespace note moved onto the named-import loop, and the mixed-form note explaining that regex's optional prefix was dropped. - test_frontend_dep_removal.py: the unquoted CSS url() note moved onto the quoted fixture, and the HTML package-name-boundary note moved off the script-src case onto the link-href case. Comments only; the gate reports code unchanged on all five files. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <michaelhan2050@gmail.com>	2026-09-04 15:31:50 -07:00
Daniel Han	827d25931b	Stop 19 test files racing on one PowerShell startup cache (#9371) ... * Stop 19 test files racing on one PowerShell startup cache Backend CI run 32341628757 on `1c3dde199` finished `284 failed, 8498 passed`. Every one of the 284 was a pwsh subprocess ending `died with <Signals.SIGABRT: 6>`, across 19 files that all read as Windows-installer regressions. None of them were. 222 of the aborts land inside a two-second window, 88 at 07:09:30 and 133 at 07:09:31, which is a mass kill of every live pwsh rather than independent per-test flakiness. The cause ------------------------------------------------------------------------ Every `-NonInteractive` startup reads and rewrites an ~83 KB `$XDG_CACHE_HOME/powershell/StartupProfileData-NonInteractive`, and XDG_CACHE_HOME defaults to `$HOME/.cache`. Under `-n 4` all four xdist workers share one HOME, so the whole job's pwsh processes race on one file and a startup that deserialises a half-written one dies before it reaches our script. `Stack overflow.` is .NET's failfast, which cannot unwind a blown stack, so it prints one line and calls abort(); that is the SIGABRT (PowerShell/PowerShell#24461). Measured twice, independently, 4000 startups per arm: run 1 shared cache dir 7/4000 died {-11: 3, -6: 4} private cache dirs 0/4000 run 2 shared cache dir 11/4000 died {-11: 10, -6: 1} private cache dirs 0/4000 Three distinct crash shapes appeared, and each names the torn file rather than our scripts: `Stack overflow.`, `System.IO.FileLoadException: The given assembly name`, and `System.ArgumentException: String cannot have zero length.` 18 deaths in 8000 shared startups, 0 in 8000 private. CI agrees from the other direction. Of the pwsh-heavy files in that run, exactly one had zero failures, tests/test_windows_amd_gpu_scan_fallback.py, and it is the only one that hands its child a private HOME, across roughly 80 startups where the run's own rate predicts about 16 failures. What is NOT established ------------------------------------------------------------------------ Neither experiment reproduces CI's rate. Roughly 20% of pwsh startups died there against 0.2 to 0.3% here, and at CI's actual `-n 4` on this box I measured 0/1200 in both arms: the race needed 48-way concurrency before it appeared at all. The likely reason is that four workers on a 4-core runner are in real contention while four threads on a 192-core box almost never overlap in the critical section, but that is reasoning and not a measurement, so treat the mechanism as proven and the magnitude as unexplained. That is also why this does not stop at removing the shared file. Three layers, in order ------------------------------------------------------------------------ 1. Remove the contended resource. One cache directory per xdist worker, fresh per session. Workers run their tests one at a time, so within a worker the startups stay sequential and the cache still does its job warm; across workers the directories are disjoint and there is nothing left to race on. Fresh rather than a stable path, because a cache torn by an earlier run would otherwise poison every later session on the same box. 2. Retry a run that produced no verdict. Three attempts, unslept, because the trigger is process startup rather than a resource that frees up. 3. Attribute what is left. A crash raises PwshInterpreterCrash naming the interpreter. Layer 1 is the fix; 2 and 3 exist because of the unexplained magnitude above. Deliberately NOT done: bounding pwsh concurrency with a lock, or giving up `-n 4`. The workflow records 806.1s to 219.7s from that flag, and the contended resource can be removed rather than rationed. The rule that keeps this honest ------------------------------------------------------------------------ A signal is not a verdict, so retrying it papers over nothing: the script never ran to its end. A normal exit is returned untouched on the first attempt whatever its code, so a pwsh that runs and gives the WRONG answer still fails with its own message. Getting that second half wrong would turn this into a way to retry real regressions into green, which is worse than the bug it fixes, so both directions are executed in tests/studio/test_pwsh_interpreter_crash_attribution.py against a real SIGABRT rather than reviewed. Mutation-tested: relaxing the crash test from `returncode < 0` to `returncode != 0` fails test_a_clean_run_with_the_wrong_answer_still_fails_with_its_own_message and test_a_clean_run_is_not_retried, which are exactly the two that guard that direction. This also generalises `_run_pwsh` from tests/studio/test_install_phase_timing.py, added earlier today for a second, signal-free shape: pwsh printing its "The PowerShell process will exit" banner and exiting normally with empty stdout. That one cannot be seen in the exit status, so it stays a text match. Verified ------------------------------------------------------------------------ tests/python/test_windows_xformers_installer.py, tests/studio/test_install_phase_timing.py, tests/studio/install/ and the new guard: 2635 passed, 3 skipped. Guard alone: 5 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drop the subprocess import the pwsh conversion left behind Source lint's import-hoist check is right: every subprocess.run in test_windows_xformers_installer.py became run_pwsh, so `import subprocess` has no references left except the one inside a comment explaining why run_pwsh is used instead. Its wording names the shape exactly -- "was used before, now unused (references re-pointed)" -- which is what a mechanical call-site rewrite leaves behind. Swept the other 18 converted files the same way with an AST pass rather than by eye. This was the only real one: the remaining hits are `from __future__ import annotations`, which every such scan reports, and a PropertyMock in test_rocm_support.py that is present on main unchanged. 42 passed. * Suppress the core dump on the forged SIGABRT tests/test_deliberate_crashes_suppress_cores.py caught this: the abort child had no PR_SET_DUMPABLE=0, so each of these aborts piped a multi-MB core to apport before the child could be reaped. The guard is right and its message names the fix. The child still exits -6 and PR_GET_DUMPABLE reads 0, so all five verdicts are unchanged. Linux-only and non-fatal elsewhere: Windows has no CDLL(None) and pipes no core, so arming it there would trade a no-op for a lost test. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com>	2026-08-20 04:22:41 -07:00
Daniel Han	6c124b6dd8	Run the repo CPU test suite on all four runner cores (#9019) ... * Fix a 500 on dictation Unload before any backend is resident stt_unload passes expected_model positionally: _, unload_stt = _stt_lifecycle() failed = await asyncio.to_thread(unload_stt, engines, model) _stt_lifecycle() returns the orchestrator's unload_stt_model when a backend is resident and stt_registry.unload when one is not. Only the first takes expected_model positionally; on the registry it sits behind a `*`: def unload(engines = None, *, wait = True, expected_model = None) def unload_stt_model(self, engines = None, expected_model = None) So with nothing loaded yet, which is what a fresh process is, Unload raises TypeError and the route answers 500. Reproduced outside pytest: peek_inference_backend() fresh process: None stt_unload -> TypeError: unload() takes from 0 to 1 positional arguments but 2 were given Pass it by keyword, which both callables accept. Found while profiling CI, not by the tests, and the tests are why: this file's two unload tests passed only because an earlier test in the full suite had left a backend resident. Standalone they failed, and in their own file they failed; they went green only inside the full serial run. That order dependence hid a live bug. The new test drives the no-backend path directly, so neither the signature nor the call site can drift back. before: 2 failed, 53 passed (file alone) after: 56 passed * Run the repo CPU test suite on all four runner cores Backend CI is the most expensive workflow in the repo at 126.7 runner-minutes per push. Install is cached at ~1.2 minutes, so almost all of it is pytest. ubuntu-latest has 4 vCPUs and this suite is CPU-bound and GPU-free, so it was running on one core of four. On a real runner the step goes 17.75 minutes to 7.67, and the failure set is unchanged: the same single pre-existing failure, serial and parallel, on the same tree in the same staging repo. Locally, on the dependency shape the workflow installs, 806.1s to 219.7s with an identical 2 failed / 8066 passed / 178 skipped / 40 subtests result. The three small pytest steps in this job stay serial: the hardware-spoof step exists because those files mutate hardware.py module globals, and all three already measure under 0.1 minutes. The four-interpreter matrix is deliberately left alone. It is the bigger prize at ~79 runner-minutes, but measured under -n 4 on a real runner it fails tests serial does not, in more than one way and not the same way twice, so it needs its order dependencies found and fixed first rather than a flag. * Pin the two unload signatures against the route's one call site The bug fixed in the previous commit exists because _stt_lifecycle returns two different callables and the route has a single call site: def unload(engines = None, *, wait = True, expected_model = None) # stt_registry def unload_stt_model(self, engines = None, expected_model = None) # orchestrator A call that suits one is a TypeError on the other, and which one runs depends on whether a backend happens to be resident, so the broken half only appears on a fresh process. That will recur the next time either signature is edited. This binds BOTH real signatures against the arguments the route actually passes. It does not demand they be identical, only that one call site can serve both. Checked against two mutations: registry grows a keyword-only param -> passes (benign, and it should) registry drops expected_model -> fails, "got an unexpected keyword argument 'expected_model'" Paired with the test beside it: that one covers the call site (route -> registry on a cold process), this one covers the two callees staying compatible. 57 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Give each node harness run its own script file Reported on this PR, and reproduced before changing anything. Every _run in test_chat_preset_builtin_invariants.py wrote the same TEMP/run.mts and then executed "node run.mts" from TEMP. Under pytest-xdist's default load distribution two of this file's nine tests can land on different workers, so one worker executes the script the other just wrote over the top of it. Pinned to four cores, running the file at -n 4: before: 5 failed / 6 failed / 5 failed / 5 failed / 6 failed (of 9) after: 9 passed, six runs out of six Serial is unchanged at 9 passed either way, which is why the full-suite runs that back this PR did not show it: at ~8000 tests across four workers these nine rarely collide, so it is a latent flake rather than a reliable failure. That is worse, not better, and enabling -n 4 is what would have started rolling the dice. A unique NAME rather than a per-call directory, which is what tests/studio/_node_harness.py::run_harness does for every other node harness here. Those scripts reach the frontend sources by a relative path counted from TEMP, so an extra directory level breaks every import with ERR_MODULE_NOT_FOUND (tried it: 9 failed). register.mjs and loader.mjs stay shared because their contents are fixed, so a concurrent rewrite writes identical bytes. Swept the other five fixed TEMP roots under tests/studio: all of them already allocate per call through tempfile.mkdtemp, directly or via run_harness. This file was the only one rolling its own. * Shorten the harness comment * Stop two more shared-state races the parallel repo job would expose Both reported on this PR, both real, both confirmed by reading the code. 1. tests/studio/test_chat_preset_builtin_invariants.py still rewrote the shared register.mjs and loader.mjs on every _run. Only run.mts became unique in the previous commit. write_text truncates before it writes, so a worker rewriting one of those while another worker's node process is importing it can hand that process an empty or partial module. The contents are constant, which is why I wrongly called a concurrent rewrite harmless: identical bytes still arrive after a truncation. They are now written through a temp file and os.replace, so every reader sees one whole version. 2. tests/python/test_no_torch_filtering.py::TestRealRequirementsFiltering had an autouse fixture that snapshotted the requirements directory and, at teardown, deleted every filtered file that had appeared since. Those files land in the REAL requirements directory, not tmp_path, so under xdist one test's teardown removes a file another worker is still reading. It now records the paths it creates and removes only those. test_chat_preset_builtin_invariants.py: 9 passed serial, 9 passed on four -n 4 runs test_no_torch_filtering.py: 65 passed serial, 65 passed on four -n 4 runs no filtered files left behind * Run the event-loop latency tests outside the parallel invocation Reported on this PR. tests/studio/load_freeze asserts upper bounds on real elapsed time -- a /health burst under 250 ms while a 600 ms blocking probe runs, and a 100-request burst under 350 ms -- and those bounds ARE the contract, so they cannot be loosened without the tests ceasing to test anything. A pytest worker descheduled by the other three inflates them. Measured before changing anything, under 3x CPU oversubscription (8 spinners pinned to the same 4 cores): 8 ms against the 250 ms bound, 38 ms against the 350 ms one, 3 runs, no failures. So the margin is wide on this box. It is not wide enough to leave alone: a runner core is several times slower, which puts the 350 ms bound within reach of an unlucky schedule. The directory is now ignored from the -n 4 run and runs in its own serial step, 20s against the ~10 minutes -n 4 saves on this job. The ignore and the step are two edits held together by nothing, and losing the step is silent -- the job stays green while 23 tests stop running. New guard tests/studio/test_backend_ci_parallel_isolation.py fails if an isolated path is missing from the parallel run's ignores, or ignored with no serial step running it, for load_freeze and for the three hardware-spoof files that were already in this shape. Deleting either half of the load_freeze change fails it; a third test pins the command scan so it cannot pass by matching nothing. load_freeze: 23 passed in 20.14s serial new guard: 9 passed, and 1 failed under each of the two mutations test_ci_shell_suite_coverage.py + test_xpu_spoof_pipeline.py: 58 passed * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Raise the matrix hang limit past the suite it was cancelling Not a review item; found while triaging why every PR in this stack carries a red check. The Backend CI matrix limit was 30 minutes, set when the step measured 14.1 to 15.0. It now measures 22.5 to 29.2, and it is cancelling: across the six open PRs on this stack, five had exactly one leg cancelled at 30m9s while its siblings passed at 25 to 29 minutes. Which leg loses is luck, so the red says nothing about the PR, and a cancelled leg burns the full 30 minutes for no signal. 45, which is what the Windows UI job already uses. This is a hang guard, not a budget, and it costs nothing until a job needs it. The real fix is running this matrix on all four cores, worth ~79 runner-minutes a push, and it stays blocked on the order dependencies this stack is working through one at a time. * Give each PowerShell probe its own script path Reported on this PR, real, and reproduced directly. _run_capturing_bytes wrote to tests/python/_{stem}_probe_{shape}.ps1, a name several tests share, so under xdist one case unlinks the script another has written before that one's pwsh child opens it. pwsh is installed on ubuntu-latest, so these do not skip there. Reverting to the shared name and running -n 4 four times: 1 failed each time. With a uuid in the name: 35 passed, 8 skipped serial 35 passed, 8 skipped on each of four -n 4 runs no probe scripts left behind * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop the pin scanner reading a generated filtered file as a second pin source Reported on this PR and reproduced. test_no_torch_filtering.py exercises install_python_stack._filter_requirements, which writes `.{stem}-filtered-XXXX.txt` BESIDE the source on purpose, so relative -r/-c includes still resolve. test_diffusers_pin.py::test_only_the_pin_file_names_diffusers scans REQ_ROOT.rglob("*.txt"), so under xdist it can see one worker's copy of diffusers-pin.txt and report it as a second source of the pin. Calling _filter_requirements on diffusers-pin.txt and then running that test fails exactly as described. It is not only an xdist problem: the function passes delete=False, so the file also survives a real install on any machine. The scan now skips dotfiles. That matches what it means -- no CHECKED-IN requirements file other than the pin may name diffusers -- rather than narrowing it: a real second source still fails it (verified with a probe file), and the production write path is untouched, since writing beside the source is the behaviour that makes includes resolve. 5 passed with a filtered file present, 1 failed with a real second source * Exempt only the generated filter files, not every dotfile Reported on this PR and right: a checked-in hidden requirements file such as .constraints.txt is a real place the pin could be overridden from, and the blanket dotfile skip took it out of the scan. Matched by the shape _filter_requirements actually writes now: a dot, the source stem, "-filtered-", and NamedTemporaryFile's 8-character suffix. generated filtered file present: 5 passed checked-in .constraints.txt naming diffusers: 1 failed --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-16 21:46:01 -07:00
Daniel Han	9c69529705	Windows: stop compiling C# for colour on hosts that already render it (#8767) ... * Windows: stop compiling C# for colour on hosts that already render it Enable-StudioVirtualTerminal is called unconditionally by install.ps1 and studio/setup.ps1, and it reaches Add-Type, which runs the C# compiler and drops a source file in %TEMP% on every install. An ANY.RUN submission of the shipped 0.1.701-beta Windows build captured that as two csc.exe processes and a "Suspicious source code drop". Under Windows Terminal there is nothing to enable: it always renders VT. Ask for that case first and skip the compile. All three conjuncts are load-bearing. WT_SESSION is inherited, so the desktop app's console-less spawn carries it into a pipe, and without the redirect check the Studio log panel would fill with escape sequences. $Host.UI.SupportsVirtualTerminal reports what the host CAN render, not whether this output buffer has ENABLE_VIRTUAL_TERMINAL_PROCESSING set, so it cannot carry the decision alone either. Nothing else moves. Outside this one function both scripts are identical to main line for line, and $script:StudioVtOk is the only value the function feeds, so the same verdict means the same bytes. The other compile stays. UnslothStudioFinalPathV2 feeds Get-StudioRuntimePathHash, which Python derives the same mutex name from byte for byte, so a managed fast path differing on case or an 8.3 name would let two installers each believe they hold the install lock. Guards: test_installer_av_shapes.py fails if the compile moves back ahead of the host check or loses a conjunct, and test_windows_setup_output_encoding.py runs this function beside the one it replaces on a real Windows host, with WT_SESSION forced set and forced empty, asserting the same verdict and the same banner bytes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Decide the redirected case without the compiler, not the Windows Terminal one Review caught that the WT_SESSION test was unsound. WT_SESSION is inherited, so a run launched from Windows Terminal into a NEW legacy console, which is what an elevated install gets, carries it with stdout not redirected and a buffer that has no ENABLE_VIRTUAL_TERMINAL_PROCESSING. SupportsVirtualTerminal reports host capability rather than the state of that buffer, so the branch would have claimed VT and printed literal escape sequences. There is no sound way to learn the current buffer's mode without GetConsoleMode, which is the compile. So decide the other direction instead: a redirected stdout is not a console, GetConsoleMode fails on a non-console handle, and the compiled path could then only return $false. Return it directly. This is provably identical rather than probably identical, and it covers the case that was actually measured: install.rs spawns install.ps1 with a pipe, so the desktop install is exactly where the compile was happening. Also drops the env plumbing from _run_console_less. It is lru_cached, so a dict argument would have raised TypeError before PowerShell was ever spawned, and the Windows parity job would have failed rather than proving anything. The parity case no longer needs it: the console-less probe IS the redirected case, so the early return is the branch under test rather than a bystander. * Reconstruct the exact merge-base function in the VT parity test The regex stripped only the guard and left the four comments above it behind, so the reconstructed predecessor was merge-base code plus comments rather than the merge-base function. Comments do not execute, so the comparison was still measuring the right thing, but a test that says it compares against the real predecessor should do that. Verified both files now reconstruct byte for byte. Also drops a stale WT_SESSION reference from an assertion message, left over from the design this PR replaced. * Tighten the comments this PR adds --------- Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-14 04:44:37 -07:00
Daniel Han	1b48147d8e	Windows: stop depending on the generated unsloth.exe console script (#8592) ... * Windows setup: install uv from a pinned release instead of running remote script text studio/setup.ps1 piped astral's install.ps1 straight into Invoke-Expression. That download-and-execute shape is the single construct AMSI providers and cloud ML scanners score hardest, and install.ps1 already replaced it with a pinned-SHA-256 archive download. Port the same implementation across. Progress goes to the pipeline rather than the console, so the quiet path swallows it exactly as it swallowed astral's installer output and the printed lines around the call site are unchanged. * Windows: stop pairing a hidden window with a bypassed execution policy The Studio shortcut launched launch-studio.ps1 with -WindowStyle Hidden and -ExecutionPolicy Bypass on the same command line. That pair is what Microsoft's own detections key on, and studio/src-tauri/src/install.rs already refuses it for the app's own launch of install.ps1. The installer writes launch-studio.ps1 itself, so the file carries no mark-of-the-web and RemoteSigned loads it. The hidden window is unchanged, so the shortcut behaves exactly as before. The generated launcher's own child launch moves to RemoteSigned for the same reason: it runs an inline -Command against an executable, where no script file is loaded and the two policies are equivalent. Also refresh a stale comment in studio/setup.ps1 that attributed the PSModulePath fix to astral's uv installer, which no longer runs in-process. * Installers: keep download-and-run command lines out of the shipped script text AMSI scans install.ps1 in full before a single line of it runs, and generic script classifiers read install.sh the same way inside the Linux bundle. Both headers rehearsed the piped web one-liner five times over, plus a scriptblock form and an execution-policy bypass, none of which anything in the scripts reads and all of which the README already documents. Point at the README instead and reword the in-body comments that quoted the one-liner as shorthand. Every printed line is untouched: the remediation text the installers show users still spells out the command in full. Same treatment for scripts/uninstall.ps1's header. * Windows: resolve process image paths with one Win32_Process query install.ps1's venv-holder probe opened a handle to every running PID through inline C# compiled at runtime. Opening a handle per process is a shape AV heuristics score hard, and it bought nothing: Win32_Process reports ExecutablePath for exactly the processes those handles could be opened against, and answers for all of them in a single query instead of once per PID. The remaining file-canonicalisation imports stay -- handle-based resolution of linked ancestors has no faithful Windows PowerShell 5.1 equivalent, and it runs on security-relevant paths. Falls back to the per-process .Path when the query is unavailable, so a degraded WMI repository degrades exactly as the old code did on a process it could not open. * Desktop: say who blocked the install when AMSI stops the script PowerShell hands the whole top-level script block to AMSI while compiling it, so a security product's verdict arrives as a parse error over the entire file before install.ps1 runs a statement: no [TAURI:ERROR] marker, no phase log, and a stderr tail the user cannot act on. unsloth#8523 shows what that looks like in the UI -- "Installation failed: + FullyQualifiedErrorId : ScriptContainedMaliciousContent". Recognise the two stable error ids on either stream and append what the user actually needs: nothing was installed, nothing was changed, it is a false positive, update definitions and retry, do not turn off endpoint protection. The raw id stays in the message, because the diagnostics report and any vendor submission both need it. Matches the id, never the message text, which is localized, and tolerates the cmdlet suffix the Invoke-Expression form carries. * Desktop: ship each bundle only the installer it can run resolve_install_script picks install.sh on unix and install.ps1 everywhere else, but the shared Tauri config bundled both into every target. The Linux AppImage therefore carried 280 KB of Windows PowerShell it can never execute -- and it is the largest script body a generic classifier walking the squashfs reads, which is where Microsoft's Trojan:Script/Wacatac.B!ml verdict on 0.1.701-beta landed. Move the resource map into the per-platform configs. The clean-machine job already fails when a Linux bundle ships no install.sh; it now also fails when one ships install.ps1, so the split cannot silently regress in either direction. The .deb scanned clean with the same payload, so this is surface reduction rather than a proven fix for that verdict. * POSIX installers: install uv from a pinned release before falling back install.sh downloaded astral's install.sh to a temp file, ran it and deleted the file; studio/setup.sh piped it straight into a shell. Both are, shape for shape, what a dropper does, and generic ML script classifiers score them accordingly -- the 0.1.701-beta Linux AppImage came back Trojan:Script/Wacatac.B!ml while the .deb carrying the same scripts came back clean. Fetch the pinned release archive and verify a hardcoded SHA-256 instead, matching what install.ps1 already does on Windows. Only the four mainstream targets are pinned: musl, armv7 and any host without a digest tool keep the path they have today, because guessing a target triple wrong would break the install outright and that costs far more than the heuristic score of the fallback. Destination, PATH handling and every printed line are unchanged, so a host that takes either path ends up in the same state it did before. * tests: pin the installer shapes antivirus heuristics score One file collecting what was removed, so it cannot drift back: no remote script run in-process, no encoded or base64 payload, no hidden window paired with a bypassed execution policy, no handle opened against another process, and no new runtime-compiled native import outside an allowlist that carries a reason for each entry that stays. The last test is the other half of the contract. Hardening must not change what a user sees, so the remediation lines the installers print -- which still spell out the web one-liner in full -- are asserted verbatim. Removing the one-liner from comments is the point; removing it from what the user is told to run would be a regression. Runs on the existing discovery-based pytest step, no workflow list to update. * release: emit a false-positive submission packet for whatever gets flagged The build job assembles a Microsoft submission packet, but only for the Windows -setup.exe. The detection that actually arrived on 0.1.701-beta was Trojan:Script/Wacatac.B!ml on the Linux AppImage, so nothing was produced for the one asset that needed it. The VirusTotal job already knows which assets were flagged and by which engines, so put the packet there: hash, size and both portals, for every flagged asset whatever platform it came from, with a note that clearance is per hash and per vendor. Engine names are not repeated -- they are third-party text and already appear escaped under Flagging engines. The gate stays advisory; this only makes acting on it take seconds. * Revert "Windows: resolve process image paths with one Win32_Process query" This reverts commit 7897865c9. tests/python/test_windows_installer_concurrency_guard.py bans Get-CimInstance and $process.Path from Get-RunningStudioVenvProcesses outright, and requires the native image-path lookup. That contract came out of #7764, which closed a set of races where the installer inferred "in use" from something other than a confirmed executable identity and blocked installs that should have proceeded. Win32_Process.ExecutablePath does answer the same question, but a wrongly blocked install costs far more than the heuristic weight of three native imports. Record the imports in the AV-shapes allowlist with that reasoning instead, and keep the ban on the process-memory APIs, which the installer has no use for. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this branch Opening comment-reduction pass over the PR diff: same intent, fewer lines. Cut hardest on the prose that restated the PR description rather than explaining the code next to it. Comments and docstrings only, verified with comment_tools.py check --strip-docstrings across every Python file in the diff. * Drop an unused helper from the uv pinned-release test * Fix three review findings on the installer hardening Stray-resource check aborted the step it was meant to assert. grep exits 1 when it selects nothing, and under this step's set -o pipefail plus the runner's bash -e that kills the assignment outright, so every correctly split .deb failed clean-machine CI before reaching the check. Both lookups take || true now: no match is the passing case for the stray one, and for install.sh it was swallowing the explicit annotation in favour of a bare exit 1. studio/setup.sh skipped astral's XDG_DATA_HOME/../bin destination tier, which install.sh, install.ps1 and studio/setup.ps1 all honour. A host that configured an XDG location got uv under ~/.local/bin instead, where no later shell looks for it. The session PATH prepend hid it at install time. The AMSI guidance claimed nothing was changed even when the block landed on the nested studio/setup.ps1, which install.ps1 launches through the same inherited pipes after the venv, PyTorch and the packages are already on disk. Split the wording on whether a [TAURI:STEP] marker has been seen: a pre-start block produces none, so the reassurance is only given where it is true. * Key the submission packet on the flagged count, not the engine list stats and results are separate fields of the same VirusTotal response, so an asset can carry a flagged count with no readable results map. The summary table reports that asset and the packet skipped it, which is exactly the one that needs a packet. Select on stats.flagged and keep the engine list for the Flagging engines section, which is correctly keyed on having engines to name. * Drop the bundle stray-resource assertion from clean-machine CI That job downloads a published release, never a bundle built from the branch, so asserting the new resource split there turns every run red until a release ships with it. The split is a property of the Tauri config, and tests/studio/test_tauri_installer_resource_contract.py already enforces it at the right layer. The || true on the install.sh lookup stays: it is what lets the explicit annotation print instead of the step dying on grep's exit 1 under pipefail. * Windows: stop depending on the generated unsloth.exe console script Fixes #8490. On Windows the `unsloth` entry point is materialised as a generated, unsigned launcher .exe. AppLocker, WDAC and Smart App Control deny it, while the venv's python.exe, a copy of the signed CPython binary, still runs. The installer died at "running unsloth studio setup" with `Program 'unsloth.exe' failed to run: An Application Control policy has blocked this file`, and because the launch throws rather than returning an exit code, it escaped Install-UnslothStudio and printed a raw NativeCommandFailed dump instead of a diagnostic. The desktop updater already solved this in update.rs by reaching the CLI through the interpreter. This applies the same idea everywhere else: the setup handoff, autostart, the shortcut launcher, the Tauri backend, auth provisioning, the install health probe, the preflight probes and the `studio run` respawn. unsloth.exe is still generated, still hardlinked to the shim, and still works. Nothing depends on it any more. Also adds `python -m unsloth_cli` as a supported entry point, and a bin\unsloth.cmd companion to the shim so `unsloth.cmd` is available where the .exe is denied. The trampoline is one string shared by install.ps1, process.rs and studio.py: import sys, os; sys.path[:1] = [x for x in sys.path[:1] if x not in ('', os.getcwd())]; sys.argv[0] = 'unsloth'; from unsloth_cli import app; app() Both halves are load bearing. argv[0] is assigned before the import because unsloth_cli decides at import time whether it is the console script, which gates the UTF-8 stream setup and the -np<N> rewrite, and it keeps typer's prog_name at `unsloth`. The sys.path[:1] filter drops the working directory entry that `python -c` adds and a console script does not, which is what lets the invocation stay off -I: -I would drop it too, but also PYTHONPATH, PYTHONWARNINGS and user site-packages, which the console script honours. Behaviour on a machine with no policy is unchanged, and that is enforced rather than asserted. tests/python/test_module_entry_point.py compares stdout, stderr and exit code between the console script, `-m unsloth_cli` and the trampoline over --version, --help, `studio --help` and two error paths. The writes are idempotent: bin\unsloth.cmd, launch-studio.ps1 and the .lnk files are content compared, so a second install changes no bytes and no timestamps. tests/studio/test_application_control_cli_fallback.ps1 pins the pieces that are easy to get wrong: the failure is classified off the exception (Win32 1260), never off $LASTEXITCODE, which no process was created to set; Start-Process gets one pre-quoted command line, since -ArgumentList joins an array with spaces and quotes nothing; and bin\unsloth.cmd only counts as an ownership marker when its contents match the shim we write, so an unrelated file of that name in a custom root cannot qualify it for removal. The new windows-application-control-ci.yml leg reproduces the report: AppLocker denies only Scripts\unsloth.exe for a standard user, a negative control proves the rule is actually enforced (the job fails loudly if the stub runs), and the full installer then has to succeed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added for the Application Control fix * Add the AGPL header to the module entry point test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drain the shim launch probe's pipes before waiting on it * Harden the cmd shim ownership marker, updater env and launcher hints * Run the Application Control CI leg without --tauri so the pinned root applies * Stub the runtime gate so the Windows launcher tests run on Windows * Isolate the advertised module route from the working directory * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix the contradictory updater env assertion and the user-site fallback * Treat a quarantined stub as a managed install on Windows * Tighten the duplicated trampoline rationale to one authoritative copy * Windows: keep a quarantined launcher and a partial migration recoverable Two follow-ups on the Application Control work. An antivirus quarantine deletes the unsigned unsloth.exe rather than denying it. The updater then found no launcher, no copy to restore, and reported a broken update, rolling back a package that was in fact fine. Absence is now excused the same way a policy denial is, but only after every recovery copy has been tried, so a launcher that could be put back still is. find_unsloth_binary_in_studio_dir accepted a bare python.exe in layout order, so an interrupted migration leaving a partial new environment beside a working legacy .venv targeted the broken one. A launcher anywhere now outranks an interpreter on its own. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Windows: let studio run start a venv whose console script was quarantined The Windows respawn goes through the interpreter and never launches Scripts\unsloth.exe, but the gate before it still required that file, so an install whose stub antivirus had taken aborted with "Unsloth venv missing 'unsloth' entry point" despite being able to run. The installed package now answers for the deleted stub, one layer down and just as cheap. POSIX still proves a CLI with the console script it execs. * CI: apply the AppLocker policy before AppIDSvc reads it The negative control watched the denied user start the stub. The job started AppIDSvc and set the policy afterwards, and the service loads the effective policy when it starts, so it was enforcing nothing; gpupdate does not make it re-read a local policy. Restart the service once the policy is in place, and retry the control while enforcement goes live, which is asynchronous and unsignalled. * Cover the uv host matrix and repeat application in the pinned-release test The pinned path picks an archive per host triple, and a wrong pick installs a binary that cannot execute, which is worse than not installing at all. Drive _uv_pinned_asset over 20 host combinations and require each one to return its own triple or decline to the fallback. Also run the installer three times over one HOME and require an identical tree, and require a stale uv at the destination to be replaced rather than joined by a second copy: the installer is re-run on every upgrade and every repair. * Windows: close the parity and old-install gaps found by the idempotency audit Five independent audits of the before/after parity bar, plus local simulations, turned up six things worth fixing. Parity, on machines with no policy at all: - Under PYTHONSAFEPATH or -P there is no implicit -c working-directory entry to strip, so sys.path[0] is whatever PYTHONPATH put there and the console script honours it. The filter removed it anyway; a PYTHONPATH starting at the working directory was measured selecting a different package through the trampoline than through the console script. - The backend start log went from a joined argument string to Rust's debug list on every platform. It is what users paste into issues. Idempotency: - The .cmd shim and launch-studio.ps1 compared decoded text, which drops a BOM and ignores case, so a BOM-prefixed shim was called unchanged and left with cmd.exe reading the BOM as part of @echo off. Both compare bytes now, launcher preamble included. - A run killed between the temp write and the rename left a temp file no later run would collect, since each names its own after its PID. Swept, skipping any whose owner is still alive. - The Application Control probe cached its verdict in :, which under irm | iex is the caller's session, so a second run in one console answered from the first run's machine state. Old installs: - An installer older than the shim directory never created one, and unsloth studio update is the only route those installs take back into install.ps1, so they never gained the .cmd. Created there now. - A migration interrupted by an open handle can split either layout. The finder now prefers a launcher with its interpreter beside it in either base, then an interpreter alone, then a launcher alone, so neither half of a split tree wins by layout order. * Pick the pinned uv archive off a positive libc check, not the absence of musl An independent audit pass found the Linux selector accepts any host whose ldd output does not say musl. That is not the same question astral's installer asks: it checks a minimum glibc and drops to its musl-static archive below it, so three hosts that worked before this branch now get a GNU binary that cannot exec, and the helper reports success so the fallback never runs. aarch64 with glibc below 2.28 (Ubuntu 18.04) x86_64 with glibc below 2.17 (RHEL 6) a musl image with no ldd at all, where the probe simply finds nothing Read the version instead, from ldd or getconf, and require it to clear astral's floor for the triple. Anything unreadable declines to the fallback. Also ask the userland for its bitness rather than trusting uname on a 64-bit kernel running a 32-bit userland, and follow astral in reading hw.optional.arm64 so a translated shell under Rosetta 2 still gets the native macOS build. Three more from the same pass: Report success only when the destination uv is executable. A copy onto a busy or read-only destination could leave a file that is not, and reporting success there skipped the fallback. Nothing is unwound on the failure path on purpose: the fallback installs over whatever is at the destination, and deleting there would take out a working uv the host already had. Clear the mark of the web on the launcher we author. WriteAllText replaces the unnamed data stream and leaves other NTFS streams alone, so a launch-studio.ps1 that somehow carried one would keep it across the rewrite, and RemoteSigned refuses a marked unsigned script. Store the security-block kind and resolve its wording in message(). stdout and stderr are read by independent threads, so a [TAURI:STEP] written before a block can be observed after it, and freezing the wording at observation time could tell a user nothing was changed on a run that had already installed PyTorch. Also require the error id to appear as the value of a FullyQualifiedErrorId field, so a scanner log that merely names it cannot attach antivirus guidance to whatever fails next. The host matrix in the shell test grows to 28 rows covering every case above, and removing the new gate fails 8 of them. install.rs gains two tests: 37 pass. * Replace a symlinked uv destination instead of writing through it Three from the review on the previous head. cp onto a destination that is a symlink follows the link, so installing over `~/.local/bin/uv -> /opt/homebrew/bin/uv` rewrote the Homebrew binary in place and left the link pointing at a file another package manager owns. Stage next to the destination and rename over it: rename replaces the link itself, and it is atomic, so a concurrent reader never sees a half-written uv either. The staging file is removed when the rename fails, so a failed run leaves no debris. Verify the Windows copy the same way the shell scripts now do. Copy-Item is non-terminating under the caller's ErrorActionPreference, so a locked or ACL-denied destination let execution reach `$haveUv = $true` and the function reported success over whatever was already there. Compare the destination against the archive we just verified, so a stale uv.exe cannot pass for the one we meant to install. install.ps1 carried the same shape and gets the same treatment. Point the header links at the heading that exists. The README has no "Install Unsloth Studio"; it is "Unsloth Studio (web UI)", whose anchor is #unsloth-studio-web-ui. Three test cases cover the symlink: the file behind the link is untouched, the link itself is replaced, and no staging file survives. Reverting the fix fails two of them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * CI: keep one Application Control negative-control log per attempt Enforcement went live on the fourth try on the hosted runner, and a single overwritten log left the evidence artifact showing a pre-enforcement attempt's output beside a passing step. * Fail the build when the uv pin drifts from a version floor Before the pin, astral's endpoint always delivered the newest uv, so raising UV_MIN_VERSION was safe on its own. It is not any more: a floor above the pin means a host with no uv gets 0.12.1 installed and then judged too old by the same script that installed it, on the one path where the pin is what runs. Two checks. All four installers must name the same uv, or which version a machine ends up with depends on which script reached it first. And the pin must clear every floor in the tree (UV_MIN_VERSION, UV_OFFLINE_MIN_VERSION, $UvMinVersion). Raising a floor past the pin fails the first, bumping one installer's pin alone fails the second. * Fix two Windows-only test failures that predate this branch test_path_identity_failure_is_reported_as_unknown failed on both shells, on main as much as here. Test-StudioPathEqual reports an unresolvable path identity through Write-StudioLine, the harness extracts the mutex helpers but not that, and these scripts run under -ErrorActionPreference Stop, so the catch path died with CommandNotFound before the test could measure anything. Extracted rather than stubbed: it is self-contained, and a stub would keep passing if the real call ever went wrong. A new check asserts every installer function the extracted helpers call is in the harness, and it runs on every platform, so the next drift cannot hide where only a Windows runner would see it. Measured on a Windows runner: main fails 24 of these, this branch fails 2, and both of those 2 are in main's set. With this, 0. * Write the shell profile entry the pinned uv path no longer gets for free The P1 here is a real regression and it took a second look to see why. install.sh decides whether to add ~/.local/bin to the user's shell profile with `case ":$PATH:"`, near the end of the run. By then this process has prepended that directory twice, once for the uv bootstrap and once for the venv, so the guard answers yes for a login shell that would answer no and the profile line is never written. That was survivable while astral's installer ran, because it wrote its own profile line and its env file. The pinned path writes neither, so on a fresh account whose login PATH lacks ~/.local/bin the install succeeds, the current shell works, and the next terminal cannot find `unsloth` or `uv`. Snapshot the inherited PATH before anything prepends to it and test the guard against that. Two more from the same review. Honour a configured uv mirror exclusively. UV_INSTALLER_GHE_BASE_URL and UV_INSTALLER_GITHUB_BASE_URL already win outright in both PowerShell installers and in astral's own; the shell path ignored them and tried the public hosts first. A restricted network sets one precisely because those hosts are unreachable, and download() has no timeout, so it would hang rather than reach the fallback. Do not let the twin of an earlier clear erase a later AMSI verdict. Clear-TauriInstallError writes one logical clear to BOTH streams (install.ps1:198) and independent threads read them, so a block observed between a clear and its own twin was discarded by the twin. Ignore a clear identical to the one just processed; a genuine later recovery carries different text and still clears. Two tests cover both directions, 39 install tests pass. * Stage the uv copy under a per-process name An audit pass reproduced a race I introduced with the symlink fix. Both POSIX helpers staged through a fixed destination-side name, so two installers targeting one directory shared it: A finishes copying the staging file B opens the same path with truncation A renames that inode into place as uv B keeps writing through its open descriptor, which is now the published uv The published uv was observable at zero bytes until B resumed, which makes the claim in the comment about a concurrent reader flatly wrong. install.ps1 is covered by its named mutex, but nothing serialises the POSIX helpers, and studio/setup.sh runs standalone on every studio update. mktemp in the destination directory instead. Each rename then publishes a file no other process can still be writing, which is what the atomicity argument needed all along. The loser cleans up its own staging file and declines, so the caller falls back rather than reporting a success it did not achieve. * Keep a default install as quiet as it was when a uv mirror misbehaves Two console regressions from the audit pass, both on paths the install still recovers from. download() runs curl -LsSf, and -S deliberately prints its own errors. The fallback ran under run_maybe_quiet, so a failed download printed nothing before; the pinned attempts run outside that wrapper, so an unreachable mirror now put two curl: (N) lines on the console of a default install that then succeeded. Redirect stderr on the speculative attempts only, leaving download() untouched for every other caller. [TAURI:WARN] is a marker level install.sh has never emitted, and the app forwards unknown markers to its progress UI verbatim (install.rs:639), so a digest mismatch would have surfaced as raw text in the desktop window. Make it a verbose only stderr line: the next mirror or the fallback still runs, so a default install has nothing to say here. Printed-string diff against the merge base is back to additions inside $(...) capture plus that one verbose-gated line, with nothing removed or changed. * Ask the installed uv whether it runs before skipping the fallback The libc gate reads a glibc version from ldd or getconf and treats that as proof a GNU binary will execute. It is not. A stripped NixOS-derived image without nix-ld reports a glibc version through getconf while its loader lives in the Nix store, so the pinned x86_64 uv asks for /lib64/ld-linux-x86-64.so.2 and gets nothing. Every static check passed, so the helper reported success, the astral fallback was skipped, and the first real uv call failed with No such file or directory. astral's installer fails its own glibc probe on that host and ships the fully static musl archive, which runs. The user went from a working uv to none. The archive is digest-verified astral uv by the time it is placed, so ask it: run --version and require it to succeed. One exec closes the whole class rather than this one host, covering a wrong triple, a loader that is not where the binary looks, and a destination we could not really write. A test drives an archive whose uv cannot execute and requires the helper to decline; removing the exec check fails it. * Pair every clear with its twin, not just the previous one install.ps1 clears after each recovered step, so a lagging reader can be several clears behind when a block lands. With clears A then B on one stream and A's twin arriving on the other after the verdict, asking only whether this is the message just seen answers no, and the delayed twin discarded the verdict the guidance exists to explain. Each logical clear emits exactly two markers, so count unpaired ones by message: the first sighting is the clear, the next pairs with it. A test drives the A, B, verdict, A', B' ordering; 40 install tests pass. * Close the exactness gaps found by ten adversarial audits Ten independent audits, each asked to falsify the claim that this is pure hardening. Six things were worth changing. - The desktop updater is isolated again. It shipped with -I, it is the one managed invocation nobody types by hand, and it decides which install gets rewritten, so a user-site unsloth_cli must not answer `from unsloth_cli import app` there. Every other call site inherits, because the console script does. - The trampoline ends in sys.exit(app()), like the generated console script, so a returned value becomes the exit status. Typer raises SystemExit itself today, but the two routes have to agree. - A launcher that could not be restored keeps its recovery copies. Judged healthy through the interpreter is not the same as repaired, and deleting the copies threw away what a later run needed. - `unsloth studio update` puts the shim directory on PATH. An installer older than that directory put the venv Scripts dir there instead, so the .cmd was written where nothing would look for it. - The console script reconfigured its streams twice off Windows, once through the import gate and once through the module-entry path. - Replacing a bin\unsloth.cmd that carries neither our marker nor our trampoline now says so. Also states the scope plainly: this answers EXE-and-DLL enforcement of the unsigned console script. A machine that also enforces AppLocker's Script collection denies .cmd and .ps1 alike, and install.ps1 would not have run there either. The two test harnesses that extract functions out of install.ps1 now assert they define everything those functions call; both had already shipped a gap that made a check pass for the wrong reason. * Ask the interpreter, not site-packages, whether the managed CLI is there The quarantine fallback accepted an unsloth-*.dist-info or an unsloth_cli/ directory as proof of a runnable CLI. Neither is: an interrupted install, or an editable install whose checkout has moved, leaves metadata with nothing to import. This gate sits in front of the headless-public strip of .bootstrap_password, so a false yes lands the exact lockout its placement exists to prevent -- a public Studio with no login page and no plaintext recovery credential. find_spec through the managed interpreter answers the question the trampoline will actually ask, with the same sys.path[0] scrub so a checkout in the caller's cwd cannot stand in for the venv. A probe that produces no verdict at all falls back to the old on-disk layout, so a half-quarantined install still starts. * Hide the import probe's console window, as every other managed probe does * Validate the staged uv before it replaces a working one My own exec check was on the wrong side of the rename. The sequence that bites: a host has a uv good enough for UV_OFFLINE_MIN_VERSION but below UV_MIN_VERSION, so the block runs with _uv_present_before true; the pinned path renames over that working binary; the --version check then fails because the loader is missing or the destination is mounted noexec; the fallback download also fails. The installer neither restores the old uv nor reports that none is available, and every later command runs the broken one. Test the staging file instead, before the rename. It sits on the destination filesystem, so it answers the noexec question too, and a binary that cannot run here never gets to replace one that could. Two tests: a working incumbent uv survives an archive whose uv cannot execute, and the rejected staging file is cleaned up. Moving the check back after the rename fails the first. * Make each managed CLI probe ask the question its launch will answer Three findings from the latest review round, one theme: a probe that stands in for a launch has to run under the same conditions as that launch, or it can pass where the launch then fails. * The quarantine gate in `studio run` asked find_spec whether unsloth_cli resolves. It resolves for an emptied unsloth_cli/ directory (find_spec calls that a namespace package), for a package whose __init__ raises, and for one whose dependencies an interrupted install never fetched, and the trampoline's `from unsloth_cli import app` fails on all three. Verified: an empty package directory in a bare venv gives find_spec True and ImportError on the import. This gate stands in front of the headless-public strip of .bootstrap_password, so a false pass there is a public Studio with no login page and no plaintext recovery credential. The probe now performs that exact import. * The updater's interpreter health check ran without isolation while the launch it predicts, build_update_command in studio/src-tauri/src/update.rs, runs under Isolation::Isolated with PYTHONHOME/PYTHONPATH cleared. A foreign checkout on PYTHONPATH could answer --version for a managed package the update had broken, and validate_launcher would keep an update the next desktop launch cannot start. _managed_cli_argv now takes the same isolated flag the Rust Isolation enum carries; the health probe is the only caller that sets it, and a test pins that it stays the only one. Every other invocation keeps PYTHON* parity with the console script. * Binary resolution, second pass. With an interrupted migration leaving an interpreter in both layouts and a launcher in neither, layout order handed back the new base even when its site-packages was empty and the legacy base still held the package. A directory test rather than an import probe: this runs on the launch path and from the capability checks, so it stays a stat. Tests: the four unimportable package shapes, the isolated/inherited argv split and its single caller, and both directions of the two-interpreter tie-break. The old whole-file "no -I anywhere" assertion is now read off the ternary, since one deliberate -I exists. * Stop the AMSI guidance claiming more than it knows Two of these are honesty defects in text a blocked user reads. "nothing was changed on this machine" is false. Rust starts a diagnostics attempt and its phase log before PowerShell is ever spawned, and spawn_script can create ~/.unsloth first, so a pre-start block has already written to disk. The honest claim is that no installation step ran. "This is a false positive" is not something the classifier can know. It proves the output carries a PowerShell error id and nothing about the script's integrity, and install.ps1 can sit in a user-writable directory, so a locally modified copy can earn a genuine verdict. Telling someone to report a correct detection to their vendor is worse than telling them to reinstall from an official package first and only escalate if an unmodified copy is still blocked. Two smaller ones from the same pass. The matcher tested for the field name and the id independently, so a line naming both in prose qualified; it now requires the id to follow the colon and end at a comma or whitespace, which is what the comment always claimed. And the clear-pairing map is bounded: legitimate producers use a small fixed label set, and child output must not be able to grow it without limit. 42 install tests pass. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honour astral's download override, and stop Unblock-File asking Three from the second audit round. Unblock-File declares SupportsShouldProcess at the default Medium impact, so a profile that sets $ConfirmPreference to Medium or Low gets a prompt from the line I added, even for a launcher that never carried the stream. -ErrorAction does not suppress a ShouldProcess prompt, and a noninteractive host turns it into an error that skips shortcut setup entirely. -Confirm:$false. UV_DOWNLOAD_URL and its older alias INSTALLER_DOWNLOAD_URL outrank the mirror variables in astral's installer, and the merge-base path inherited that because it ran astral's script. All four implementations now honour them first and exclusively. My earlier comment argued they point at a version the pin would reject, but that reasoning had it backwards: a host sets one because it cannot reach the public endpoints, so ignoring it meant public egress first and, with no timeout on the download, a hang instead of a fallback. The pin still applies, so a source serving a different build fails the digest and the caller falls back to astral's installer, which honours the same variable. chmod 0755 on the staging file rather than +x. cp gives it the umask default and +x then adds execute only where the umask allowed read, so a umask of 077 left uv unusable for every other account on a shared machine. astral ships them 0755. Four checks pin the override precedence across all four installers and the mode across both shell ones, with the behaviour verified against a stubbed downloader. * Validate uv before it replaces an incumbent on Windows, and bound the probe install.ps1 and studio/setup.ps1 copied the extracted uv.exe straight over the destination and only asked whether it ran afterwards. A host with a working older uv and a policy (AppLocker, WDAC, endpoint protection) that refuses the new one was left with neither. Run the extracted binary where it landed first, then keep a copy of the incumbent across the publish and restore it if the published copy will not run, since Windows has no atomic replace for a file that may be open. The probe itself is bounded: Start-Process with a 20s WaitForExit and redirected streams, and on POSIX no stdin plus a 20s ceiling where timeout exists. A binary this installer just downloaded must not be able to hang an unattended install by prompting or by never exiting. install.sh and studio/setup.sh also published the pinned uvx after rejecting the pinned uv, leaving a pairing that is never built or tested. A uv that fails to stage, copy or run now abandons the whole placement. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Verify each uv mirror, and persist PATH when the account has no rc file Both PowerShell installers checked the archive digest once, after the download loop had already broken out. A captive portal or a proxy answering 200 with its own body is a successful download by every measure Invoke-WebRequest has, so the first mirror consumed the only attempt and the second, healthy one was never tried. The digest now decides whether a mirror counts as served. install.sh picked a shell profile from .zshrc, .bashrc or .profile and did nothing when none existed. A fresh account has none: astral's installer used to create its own PATH setup there, the pinned path does not, so the next terminal resolved neither unsloth nor uv. Fall back to creating ~/.profile, which every POSIX login shell reads. The existing content guard keeps it written once. * Remove the install.sh a Windows upgrade would otherwise keep forever Windows bundles now carry only install.ps1, but NSIS writes the current resource manifest and deletes nothing, and the uninstaller deletes only what is in that manifest. An in-place upgrade from a release that bundled both installers left install.sh in $INSTDIR permanently, which also made the non-recursive RMDir "$INSTDIR" fail at uninstall. The pre-install and pre-uninstall hooks now delete it, so the population most likely to upgrade actually gets the split. Also silence the speculative mktemp -d in the pinned uv path: its failure falls back to astral's installer, so an unusable TMPDIR printed a line the user could not act on and that the merge base did not print. * Remove the pinned uv temporaries when an install is interrupted The pinned path unpacks a 40 MB archive into a work directory and stages the binary next to the destination, but only cleaned both up when the helper returned normally. A Ctrl-C in between left the archive behind and left a staging file inside a directory that is on PATH. Both paths are now published to the exit and signal traps as they are created and cleared when the helper releases them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Persist the PATH the way each shell actually reads it, and fail a half-published pair Four follow-ups from review: A uvx that the archive carried but that could not be staged or renamed left uv published next to a stale or missing uvx and still reported success, skipping the fallback that would have installed both. Either half failing now fails the placement, in install.sh and studio/setup.sh. studio/setup.sh had none of the interrupt cleanup install.sh gained: a Ctrl-C left the unpacked archive behind and a staging file inside a directory on PATH. It now owns HUP, INT, TERM and EXIT for the duration of the pinned install and hands them back on the way out. fish sources none of the POSIX rc files, so the ~/.profile fallback was a no-op for a fish user. The persistence helper writes a conf.d drop-in with fish_add_path there, and honours ZDOTDIR for zsh. UV_INSTALL_DIR, UV_UNMANAGED_INSTALL, XDG_BIN_HOME and XDG_DATA_HOME can put uv somewhere other than ~/.local/bin, and astral's installer wrote a PATH line for whichever it picked. The pinned path now persists its own destination too, with UV_NO_MODIFY_PATH honoured as astral honours it. * Make the Windows uv publish a real transaction, and quote persisted paths The companion copies ran bare: under install.ps1's Stop preference a locked or ACL-denied destination threw past the rollback and left a mismatched set with the backups still on disk, and under setup.ps1's Continue preference it kept a stale companion and reported success. Both now copy under -ErrorAction Stop inside the transaction, so any failure unwinds like the others. A failed restore also used to delete the backup anyway, which is the one path in this block that could leave the host with less than it started with: the two things that make a restore fail, an open incumbent and a denied ACL, are the same two that made the replace risky. The backup is now kept and named. fish takes an unquoted path with a space as two directories, neither of which exists, so the drop-in single-quotes it; and the rc line is written inside double quotes, so a uv directory holding a dollar or a backtick is escaped. The second test caught a doubled backslash in the escaper itself. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this branch Comments only, no code touched: 161 comment lines become 106 across install.sh, install.ps1, studio/setup.sh, studio/setup.ps1 and the NSIS hooks. Each one keeps the reason it was written for, said once. Verified with the PowerShell AST parser, sh -n and bash -n, the 50-check uv pinned release suite and 114 installer tests, and by confirming the diff contains no non-comment line. * Tighten the install.rs comments too Comments only: 35 lines become 27, each keeping the reason it was written for. 42 install tests pass and the diff contains no non-comment line. * Abort on a companion that cannot be backed up, and pair clears by stream A uvx.exe that could not be copied aside, because it is locked or its ACL denies reads, was skipped and the new uv.exe published anyway, so the function reported success with a mismatched pair and the fallback never ran. Any backup failure now fails the placement and runs the rollback, in install.ps1 and studio/setup.ps1. The ERROR_CLEAR pairing keyed only on the message, so two real clears of one label on one stream were taken for a clear and its twin. That happens: _install_torch_default_index emits its recovery during the install and again during the ROCm repair. A verdict landing between them was then erased by the genuinely later clear arriving on the other stream. The map is keyed by stream as well, so only the opposite stream's copy can consume a pending marker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Do not fail an install because the uv probe could not get an answer Three clean-machine CI legs that pass on main failed on this branch: arm64 and two Windows containers, all three with winget unavailable, which is the only condition under which the pinned fallback runs. Each downloaded the right asset, passed the digest, and then failed the probe. Start-Process -NoNewWindow with redirected streams does not behave in a container or on the arm64 image the way it does in a desktop session, and a boolean probe reported that as a broken binary and aborted the install. The probe is now tri-state. Only the binary answering non-zero is a failure. A launch that throws or a wait that times out is inconclusive, and since the digest already proved the bytes are astral's pinned release, an inconclusive probe publishes as the pre-pin code did. Every path prints why, with the captured stderr and the exit code, so the next occurrence is not opaque. Also from review: the POSIX path now stages both binaries and publishes them together with the incumbents saved aside, so a failed uvx rename restores the uv it replaced instead of leaving a new uv beside a stale uvx; the Windows rollback records the destination before the copy that can truncate it; and UV_UNMANAGED_INSTALL suppresses the profile write, as it does for astral. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Give setup.sh the same pair publish and PATH persistence as install.sh studio/setup.sh published uv and then uvx one after the other, so a failed uvx rename left a new uv beside the host's stale one, and the remote fallback can be unavailable. It now stages both, validates uv, and publishes the two renames back to back with the incumbents saved aside, restoring them if the second fails. setup.sh is also run directly for local and Colab setup, where astral's installer used to write the profile line for whichever destination it chose. Without one the PATH export died with that shell and every later run reinstalled uv. It now persists its own destination, with fish handled on its own terms and both of astral's opt-outs honoured. * Treat an empty uv exit code as no verdict, not as a failure The arm64 clean-machine leg still failed on the tri-state probe, and the diagnostic that came with it said why: "uv --version exited ." with no number. WaitForExit(ms) can return before the exit code is cached, so ExitCode was empty and an empty value is not 0, which read a working uv as broken. The parameterless WaitForExit settles it and returns at once because the process has already exited, and a code that is still missing is inconclusive rather than a failure, which is the same rule the launch and timeout paths already follow. Verified against pwsh that a real non-zero exit and a real launch failure still classify as failed and unknown respectively. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Eight review fixes across the uv publish and PATH persistence The fish escaper in studio/setup.sh reached sed as an invalid expression, so a fish user running setup directly would have had setup killed under set -e right after uv was published. It now matches the one in install.sh, and the test runs both escapers rather than reading them. An incumbent that cannot be hard-linked or copied cannot be restored either, so publishing over it would be a one-way move. Both shells now decline. Writing that test turned up that my own rollback deleted both incumbents when nothing had been published, since the no-predecessor branch cannot tell the two cases apart; the rollback is now reached only after a publish was attempted. A rollback with no predecessor removes the binary it published, rather than leaving half a pair the host never had. A signal between the two renames left the undo copy as the only reference to the incumbent, and the handler deleted it. It restores it now, in both shells. setup.sh prepended ~/.local/bin unconditionally after a successful pinned install, so a stale uv there could shadow a custom UV_INSTALL_DIR destination and the rest of setup would run the wrong one. That prepend is now only for astral's installer, which is what writes there. PATH entries are compared literally rather than as case patterns, so a destination holding *, ? or [ is not mistaken for an unrelated entry. On Windows, a .unsloth-old left behind by a failed restore is the only copy of a working uv, and the next run reused that exact name. It takes a distinct one. * Keep the pinned uv first on PATH, and only count an active profile entry install.sh prepends ~/.local/bin after the uv bootstrap, and astral's env file does too, so a custom UV_INSTALL_DIR destination was pushed behind a stale uv sitting in the home directory and every bare uv below picked the wrong one. The pinned destination goes back in front. setup.sh had the same shape and was fixed in defd2292a. The profile check treated any occurrence of the destination text as proof the PATH entry was already there, so a commented-out old export, or /opt/uv-old when the destination is /opt/uv, suppressed the write and left the next shell without uv. Comments are stripped and the directory has to appear as a whole entry. * Close five review findings on the quarantine and stubless paths * The installer still required Scripts\unsloth.exe to exist, and aborted the whole install when it did not. That reasoning held for a policy, which denies the file and leaves it on disk, but not for antivirus, which quarantines it out of a venv that still runs, and nothing past that point executes it: the setup handoff, the shortcuts and bin\unsloth.cmd all go through the interpreter. It refused to install or repair Studio for exactly the machines this change is for. Absence now asks the interpreter for --version through the trampoline, and only a venv that cannot answer fails, with the same older-unsloth guidance. * The import probe's no-verdict fallback is now split by cause. A timeout keeps the on-disk layout, because slow is not broken: a cold venv under an antivirus scan is exactly that, and the re-exec has no timeout of its own. A failure to START the interpreter fails closed, because the re-exec runs that same interpreter and will fail the same way, and the caller strips .bootstrap_password before re-execing on a headless public launch. * The updater's interpreter fallback used the launcher's 10s timeout for a call that has to import the entire CLI package. That is the work the import probe's 60s ceiling is deliberately generous for, and under the antivirus scan this path exists to survive the short one would call a healthy update broken and roll it back, once per recovery candidate. * Binary resolution now accepts an unsloth-*.dist-info alongside the package directory when ranking stubless venvs, matching _managed_cli_site_packages_ layout. A PEP 660 editable install leaves a .pth and a dist-info and no unsloth_cli/ at all, so the directory test alone ranked a working legacy venv below an empty new one. * managed_bin_fingerprint required fs::metadata on the launcher, which the stubless layout deliberately reports as a path that does not exist, so the capability cache could be neither read nor written and every preflight paid both probe subprocesses again. It falls back to python.exe, which is what starts the CLI there, while the cache key stays the launcher path. Tests: the fail-closed/fallback split in both directions, the timeout contract and that the two constants differ, the editable-install ranking with an unrelated dist-info as the negative control, the stubless fingerprint and its invalidation, and the installer gate through the extracted AST harness. * Gate the NSIS tidy-up, and remove an orphan uv on signal The pre-install hook runs before the user can still cancel, and $INSTDIR can be a directory they picked in the GUI, so deleting install.sh there could take a file that was never ours. Both hooks now only act where our own executable already is. A signal between the two renames restored a predecessor but did nothing when there was none, leaving a 0.12.1 uv beside whatever uvx the machine had. It now removes what it published, which is what the ordinary rollback already does. * Write the uv PATH entry to every startup file astral's installer wired astral's uv installer wires ~/.profile, each of .bashrc, .bash_profile and .bash_login that exists, .zshrc or .zshenv under ZDOTDIR, and a fish drop-in under ~/.config. Replacing that installer with a pinned archive meant the PATH entry only reached the one file for whichever shell happened to be running, so a bash user whose .bash_profile does not source .bashrc, a /bin/sh login, or anyone who later switched shells would have no uv on PATH where they used to. Both POSIX installers now write the same set, once each, with the existing whole-entry check keeping a re-run idempotent. Files that do not exist are not created, apart from ~/.profile, which astral creates too. * Cut the uv publish back to what the common case needs The rollback machinery that grew over the review rounds covered cases a user is very unlikely to meet: an incumbent that cannot be hard-linked, a signal landing between two renames, a restore that itself fails, a second installer racing the first. It was 281 net lines, and every finding in the last two rounds was in it rather than in the hardening. What stays is what the common case needs. POSIX stages both binaries, runs the staged uv, and publishes the pair with two renames; a failure anywhere before them leaves the destination untouched, and the caller falls back to astral's installer exactly as before. Windows probes the extracted uv.exe before touching the destination, then copies the three under -ErrorAction Stop and re-checks the digest at the destination. The staging files are still removed on a signal, since they live in a directory that is on PATH. 64 shell checks and 114 installer tests cover the rest. * Match the exact fish entry, and let a UNC launcher load The fish drop-in is the only thing that puts uv on a fish user's PATH, since fish reads none of the POSIX files, and its check treated any occurrence of the directory as proof: /opt/uv-old suppressed /opt/uv. It now matches the exact fish_add_path line it would write. A launcher on a UNC share is a remote script to PowerShell, and RemoteSigned refuses an unsigned one, so a roaming profile got a shortcut that exits without starting Studio. That case, and only that case, uses Bypass, and drops -WindowStyle Hidden with it so the pair the detections key on never appears. * Wire every startup file on a DEFAULT install too, and give setup.ps1 a fallback The all-profile PATH write was gated on the uv destination differing from ~/.local/bin, which is exactly where a normal install puts it, so every ordinary machine still got the single-file write the shim path has always done. Three independent audits found this. The gate is gone, and the idempotency check now also matches the $HOME-relative spelling the shim block writes, so the default case does not end up with two lines for one directory. studio/setup.ps1 replaced astral's installer with the pinned archive and had nothing to fall back to. A failed pinned install therefore left UseUv false and silently ran torch, bitsandbytes, Triton and the rest through pip: a different resolver, not just a different download. winget is the fallback, as install.ps1 already does, rather than the remote script this branch exists to remove. * Make the quarantine case survive root inference, PATH and the reset hint Three more from review, all the same shape: a Windows path that still treats the generated unsloth.exe as the only evidence of an install. * Root inference. _looks_like_installer_managed_studio_home accepted share/studio.conf or bin\unsloth.exe, and only install.sh writes studio.conf, so on a custom-root Windows install the quarantinable launcher was the only sentinel there was. Once antivirus took it, STUDIO_HOME fell back to ~/.unsloth/studio and every studio subcommand read and wrote the wrong tree while reporting success. bin\unsloth.cmd now counts, validated against the same marker pair and 8 KB ceiling Test-UnslothCmdShimFile and the uninstaller's recursive-delete guard use, because this decides which installation the CLI manages and the directory is on PATH. * The PATH gate took any leaf named unsloth.cmd as a usable launcher. Write-UnslothCmdShim warns and leaves an unwritable file alone, so a foreign shim in a custom root survives the run, and counting it put its directory on PATH and advertised someone else's command as the policy-safe way in. It goes through Test-UnslothCmdShimFile now. * The reset-password hint always advertised `-I -m unsloth_cli` on Windows. -I implies -s, so a pip install --user install was handed a command that cannot find its own package, and the person reading it is by definition already locked out. It now checks whether the package is inside the interpreter's prefix and otherwise prints the bootstrap unsloth_cli/__main__.py documents for exactly this case, which carries no double quote and so wraps identically for cmd and PowerShell. Tests: root inference through a validated .cmd with four rejected impostors, an oversized shim, POSIX unchanged; the PATH gate as a source contract; and a new studio/backend/tests/test_reset_password_command.py covering both interpreter shapes, the spaced-path fallback, the prefix check, and drift between the bootstrap here and _WINDOWS_CLI_ENTRYPOINT. * [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 <danielhanchen@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com>	2026-08-13 07:54:51 -07:00
Wasim Yousef Said	f567ae8f39	Studio: skip redundant packaged frontend rebuilds (#8326) ... * Desktop: skip frontend rebuild during updates * Tests: tolerate rustfmt in updater UTF-8 contract * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: use packaged frontend for PyPI installs * Studio: keep the packaged frontend skip off source checkouts STUDIO_LOCAL_INSTALL records where the Python package came from, not which tree setup runs out of. An editable overlay separates the two: with UNSLOTH_CI_SOURCE_OVERLAY, or in a venv left editable by an earlier --local run, the mode stays 0 while SCRIPT_DIR is a checkout whose dist is a stale build artifact rather than a release one. The skip then serves that stale dist and a source change silently never reaches the browser, which is the outcome the overlay legs of clean-machine-install-ci exist to catch. A wheel ships no top-level files, so a pyproject.toml next to studio/ marks the tree as source. Require its absence before trusting the packaged dist; site-packages installs are unaffected and still skip. Also check the Tauri branch before the packaged one in setup.ps1 so a desktop update reports the same reason it reports on POSIX. Covered by new cases in tests/sh/test_packaged_frontend_skip.sh and tests/studio/test_node_decision.ps1. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>	2026-08-11 03:02:17 -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
Daniel Han	d8effae0d5	Studio: fix the Windows desktop setup log mojibake and double-printed steps (#8083) ... * Studio: fix the Windows desktop setup log mojibake and double-printed steps The desktop 'Getting things ready...' log rendered as: ?? Unsloth Studio Setup <52 replacement chars> gpu none (chat-only / GGUF) gpu none (chat-only / GGUF) Encoding. studio/setup.ps1 never set [Console]::OutputEncoding, so Windows PowerShell 5.1 encoded redirected output with the OEM code page while the desktop app decodes the pipe as UTF-8 (String::from_utf8_lossy in src-tauri/src/install.rs). That corrupts two different ways: the sloth U+1F9A5 has no OEM representation so PowerShell substitutes one '?' per UTF-16 surrogate, and the rule U+2500 does have one, so it becomes a bare 0xC4 byte that is invalid UTF-8 and surfaces as U+FFFD. Both entry scripts now set the console encoding, $OutputEncoding, PYTHONUTF8 and PYTHONIOENCODING before the first write, and Refresh-Environment can no longer reload the two Python vars back over ours mid-run. The patch is ASCII-only: these files are UTF-8 without a BOM and 5.1 parses those as ANSI. Duplication. step/substep wrote through Write-Host AND a console-handle mirror. The mirror's comment assumed Write-Host does not survive the process chain; it does, because the CLI spawns setup.ps1 as -Command "& '...' *>&1" (unsloth_cli/commands/studio.py), which merges the Information stream into stdout deliberately. The sink is now resolved once and exactly one is used: redirected writes to the console handle, interactive writes to Write-Host. Splitting. step composed one logical line from two Write-Host calls using -NoNewline, and a redirected consumer turns each Information record boundary into a line break. Both scripts now emit one composed record; install.ps1 needs this most, having no mirror to fall back on. Rust children on Windows get PYTHONUTF8/PYTHONIOENCODING too, since install.rs, update.rs and process.rs all decode their output as UTF-8. The readers stay lossy on purpose -- strict decoding would turn display corruption into an installation failure. Tests: a Pester suite auto-discovered by the existing pester job, and a pytest byte-level probe that runs real PowerShell in both the -File and -Command launch shapes and asserts on raw bytes. Verified to fail against the unfixed tree (9 Pester and 13 pytest failures) rather than merely passing. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bind a UTF-8 writer with no console, and pass -X utf8 to the isolated child Two holes in the previous commit, both on the exact path the desktop app takes. [Console]::OutputEncoding P/Invokes SetConsoleOutputCP, which needs a console handle. Under CREATE_NO_WINDOW there is none, so it throws, and it drops the cached writer BEFORE throwing while assigning OutputEncoding only after. Console .Out therefore rebuilt on the old code page. Swallowing the exception was not enough once redirected step/substep use Console.Out as their only sink, so the catch path now binds an explicit UTF-8 StreamWriter over OpenStandardOutput. build_update_command launches Python with -I, which implies -E, so that process ignores every PYTHON* variable and PYTHONUTF8/PYTHONIOENCODING never reached it. Pass -X utf8 as a switch instead. The env vars stay for its descendants. https://docs.python.org/3/using/cmdline.html#cmdoption-I * Bind the UTF-8 writer to stderr as well when there is no console The no-console fallback repaired Console.Out only. Tauri pipes stderr through the same lossy UTF-8 decode (install.rs) and emits it to the same UI log, and InstallFailureContext builds the user-facing failure message from those lines, so a PowerShell error carrying a non-ASCII path still arrived as U+FFFD. install.ps1 also writes its Clear-TauriInstallError markers there. * Tighten the comments added by this PR Comments only, no code change. Verified with the PowerShell AST tokenizer for both .ps1 files and the Pester suite (token streams identical with Comment and NewLine excluded), comment_tools.py for the Python test, and a code-only diff for update.rs. * Update the Windows command assertion for the added UTF-8 flags windows_update_command_uses_python_not_replaceable_console_stub asserts the exact argument vector, so adding -X utf8 broke it. The Windows cargo test job in studio-tauri-smoke.yml runs it; the Linux job skips it under cfg(windows), and cargo check type-checks tests without running them, so neither the org Linux run nor the staging check caught it. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-07 01:18:35 -07:00