From a9ab8aead0ab7a90e80147ca6698542bc2529223 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Wed, 29 Jul 2026 02:27:13 +0000 Subject: [PATCH] Tighten the interrupted-install comments --- .github/scripts/interrupt-install.ps1 | 58 +++---- .github/scripts/interrupt-install.sh | 54 +++---- .github/scripts/interrupted_install_probe.py | 131 +++++++--------- .github/workflows/interrupted-install-ci.yml | 155 +++++++++---------- 4 files changed, 175 insertions(+), 223 deletions(-) diff --git a/.github/scripts/interrupt-install.ps1 b/.github/scripts/interrupt-install.ps1 index 917cef32cd..fd763eaedb 100644 --- a/.github/scripts/interrupt-install.ps1 +++ b/.github/scripts/interrupt-install.ps1 @@ -4,11 +4,9 @@ # Windows counterpart of interrupt-install.sh: run install.ps1 and kill it partway # through, reproducing a user quitting the desktop app mid-install. # -# Windows has no process groups, which is why the app carries windows_job.rs -- a Job -# Object is what makes "kill the installer and everything it spawned" work there. This -# script kills the whole process TREE for the same reason: killing only the leader -# leaves uv/python children to finish the dependency pass, and the interruption would -# prove nothing. +# Windows has no process groups, which is why the app carries windows_job.rs. This script +# kills the whole process TREE for the same reason: killing only the leader leaves +# uv/python children to finish the dependency pass, proving nothing. # # Usage: # pwsh -File .github/scripts/interrupt-install.ps1 -Marker 'studio deps' ` @@ -26,13 +24,11 @@ $ErrorActionPreference = 'Continue' New-Item -ItemType Directory -Force -Path (Split-Path -Parent $LogPath) | Out-Null Set-Content -Path $LogPath -Value '' -Encoding utf8 -# Stand in for the desktop app, which creates this before spawning the installer and -# clears it only on a terminal outcome (install.rs). We kill install.ps1 directly -# rather than driving the real app, so without this the marker #7490 relies on is -# absent for a reason that has nothing to do with #7490 -- which is exactly what the -# Windows legs reported. Both locations because the Rust side hardcodes -# ~/.unsloth/studio while CI overrides UNSLOTH_STUDIO_HOME. Never cleared: being -# killed is the whole point. +# Stand in for the desktop app, which writes this before spawning the installer +# (install.rs). We kill install.ps1 directly, so without it the marker #7490 relies on is +# absent for a reason unrelated to #7490 -- exactly what the Windows legs reported. Both +# locations because the Rust side hardcodes ~/.unsloth/studio while CI overrides +# UNSLOTH_STUDIO_HOME. Never cleared: being killed is the whole point. foreach ($dir in @($env:UNSLOTH_STUDIO_HOME, (Join-Path $HOME '.unsloth\studio'))) { if ([string]::IsNullOrWhiteSpace($dir)) { continue } try { @@ -41,17 +37,13 @@ foreach ($dir in @($env:UNSLOTH_STUDIO_HOME, (Join-Path $HOME '.unsloth\studio') } catch { Write-Host "[interrupt] could not seed install marker in ${dir}: $_" } } -# Run the installer in its own host so stdout can be redirected to the log while we -# poll. That host is WINDOWS PowerShell, not pwsh: the desktop app spawns the bundled -# install.ps1 as `powershell.exe -NoLogo -NoProfile -NonInteractive -WindowStyle Hidden -# -ExecutionPolicy Bypass -File` (install.rs:325-339), so 5.1 with those flags is the -# only host a real desktop install ever uses. Every other Windows job in .github runs -# install.ps1 under the runner's pwsh 7, which leaves the installer's behaviour on 5.1 -# -- .NET Framework rather than .NET, OEM/ANSI console encoding rather than UTF-8, -# different native-command and OSArchitecture reporting -- covered by nothing. An -# interruption test that runs a different interpreter than the app cannot claim to -# reproduce what the app does. The driver itself stays under pwsh; only the installer -# child and the repair re-run change. +# Its own host, so stdout can be redirected to the log while we poll. That host is +# WINDOWS PowerShell 5.1, not pwsh, with install.rs:325-339's exact flags: that is the +# only host a real desktop install ever uses, and every other Windows job in .github runs +# install.ps1 under pwsh 7, leaving 5.1 behaviour (.NET Framework, OEM/ANSI console +# encoding, different native-command and OSArchitecture reporting) covered by nothing. +# The driver itself stays under pwsh; only the installer child and the repair re-run +# change. $argList = @( '-NoLogo', '-NoProfile', '-NonInteractive', '-WindowStyle', 'Hidden', @@ -65,8 +57,8 @@ $proc = Start-Process -FilePath 'powershell.exe' -ArgumentList $argList ` Write-Host "[interrupt] installer pid=$($proc.Id) marker='$Marker' deadline=${KillAtSeconds}s" function Stop-Tree([int]$RootId) { - # Depth-first: children before parents, so a parent cannot respawn a child we already - # killed. CIM gives us the parent link Windows does not expose via process groups. + # Depth-first, so a parent cannot respawn a child we already killed. CIM gives the + # parent link Windows does not expose via process groups. $kids = @(Get-CimInstance Win32_Process -Filter "ParentProcessId=$RootId" -ErrorAction SilentlyContinue) foreach ($k in $kids) { Stop-Tree ([int]$k.ProcessId) } try { Stop-Process -Id $RootId -Force -ErrorAction Stop; Write-Host "[interrupt] killed pid=$RootId" } @@ -96,15 +88,13 @@ if (-not $killed -and -not $proc.HasExited) { if (-not $reason) { $reason = 'dea if ($killed) { Write-Host "[interrupt] killing process tree of $($proc.Id) ($reason)" Stop-Tree $proc.Id - # Any straggler uv/python that reparented away from the installer. The old sweep - # matched nothing: UNSLOTH_STUDIO_HOME arrives as `D:\a\r\r/.studio-home` - # (github.workspace joined with a forward slash) while Process.Path is all - # backslashes, so the literal -like missed even the venv's own python, and uv is - # never under the studio home anyway (install.ps1 takes it from winget or - # astral.sh). Normalise the separators, and take uv by name since the runner is - # ephemeral and runs no other uv. - # Under --tauri there is no UNSLOTH_STUDIO_HOME, so fall back to the root - # install.ps1 uses then, or the sweep would only ever see uv. + # Any straggler uv/python that reparented away from the installer. The old sweep matched + # nothing: UNSLOTH_STUDIO_HOME arrives as `D:\a\r\r/.studio-home` (github.workspace + # joined with a forward slash) while Process.Path is all backslashes, so the literal + # -like missed even the venv's own python. Hence the separator normalisation, and uv by + # name (it lives outside the studio home, and the ephemeral runner has no other uv). + # Under --tauri there is no UNSLOTH_STUDIO_HOME, so fall back to the root install.ps1 + # uses then, or the sweep would only ever see uv. $studioRoot = if ([string]::IsNullOrWhiteSpace($env:UNSLOTH_STUDIO_HOME)) { Join-Path $HOME '.unsloth\studio' } else { $env:UNSLOTH_STUDIO_HOME } $homeNorm = if ([string]::IsNullOrWhiteSpace($studioRoot)) { $null } diff --git a/.github/scripts/interrupt-install.sh b/.github/scripts/interrupt-install.sh index c6cfe9b0f8..c24382c56b 100755 --- a/.github/scripts/interrupt-install.sh +++ b/.github/scripts/interrupt-install.sh @@ -2,13 +2,10 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. # -# Run install.sh and SIGTERM it partway through, reproducing what the desktop app does -# when the user quits mid-install: main.rs cleanup_child_processes() -> -# install::stop_install() -> kill the installer PROCESS GROUP (install.rs:798-807). -# -# Killing only the leader would leave `uv`/`python` children running and finishing the -# dep pass, so the interruption has to target the group -- otherwise the test quietly -# proves nothing. +# Run install.sh and SIGTERM it partway through, reproducing a user quitting the desktop +# app mid-install: main.rs cleanup_child_processes() -> install::stop_install() -> kill +# the installer PROCESS GROUP (install.rs:798-807). It must be the group: killing only +# the leader leaves `uv`/`python` children to finish the dep pass, proving nothing. # # Usage: bash .github/scripts/interrupt-install.sh "" "" [-- install args] # regex to wait for in the install log before killing, e.g. "studio deps" @@ -28,12 +25,11 @@ KILL_GRACE="${KILL_GRACE:-10}" mkdir -p "$(dirname "$LOG")" : > "$LOG" -# Stand in for the desktop app, which creates this before spawning the installer and -# clears it only on a terminal outcome (install.rs). We kill the installer directly -# rather than driving the real app, so without this the marker #7490 relies on is -# absent for a reason that has nothing to do with #7490. Written to both locations -# because the Rust side hardcodes ~/.unsloth/studio while CI overrides -# UNSLOTH_STUDIO_HOME. Deliberately never cleared: being killed is the whole point. +# Stand in for the desktop app, which writes this before spawning the installer +# (install.rs). We kill the installer directly, so without it the marker #7490 relies on +# is absent for a reason unrelated to #7490. Both locations because the Rust side +# hardcodes ~/.unsloth/studio while CI overrides UNSLOTH_STUDIO_HOME. Never cleared: +# being killed is the whole point. for _marker_dir in "${UNSLOTH_STUDIO_HOME:-}" "$HOME/.unsloth/studio"; do [ -n "$_marker_dir" ] || continue mkdir -p "$_marker_dir" 2>/dev/null || continue @@ -41,7 +37,7 @@ for _marker_dir in "${UNSLOTH_STUDIO_HOME:-}" "$HOME/.unsloth/studio"; do done # Job control puts the child in its own process group, so $! is the pgid leader and -# `kill -- -$!` reaches every descendant -- matching the Rust side. +# `kill -- -$!` reaches every descendant, matching the Rust side. set -m bash install.sh "$@" > "$LOG" 2>&1 & PID=$! @@ -56,13 +52,12 @@ for i in $(seq 1 "$KILL_AT_SECONDS"); do break fi if [ -n "$MARKER" ] && grep -qE "$MARKER" "$LOG" 2>/dev/null; then - # Let it get a beat into the step, so the kill lands mid-work rather than on the - # boundary where the step has not started touching the venv yet. + # A beat into the step, so the kill lands mid-work rather than on the boundary + # before the step has touched the venv. sleep "${KILL_AFTER_MARKER_SECONDS:-3}" - # ...but a late step whose work is already cached can FINISH inside that beat. - # Recording marker-hit before the sleep handed the landing assertion a COMPLETED - # install: the signal reached no process, the probe read HEALTHY, and the leg - # passed green having interrupted nothing. Set the reason after, not before. + # ...but a cached step can FINISH inside that beat. Recording marker-hit before the + # sleep handed the landing assertion a COMPLETED install: the signal reached no + # process and the leg passed green having interrupted nothing. if ! kill -0 "$PID" 2>/dev/null; then reason="exited-during-marker-delay" break @@ -86,11 +81,10 @@ if [ "$killed" = "true" ]; then kill -0 "$PID" 2>/dev/null || break sleep 1 done - # Unconditional, and to the GROUP. The leader can exit on SIGTERM while a uv or - # python descendant ignores it or is mid-shutdown; `kill -0 "$PID"` then reported - # the leader gone, this escalation was skipped, and `wait` reaped only the leader, - # leaving that descendant free to finish the dependency pass while the probe ran. - # Signalling an already-empty group is a no-op. + # Unconditional, and to the GROUP. The leader can exit on SIGTERM while a uv or python + # descendant does not; gating on `kill -0 "$PID"` skipped this escalation and left that + # descendant free to finish the dependency pass while the probe ran. Signalling an + # already-empty group is a no-op. echo "[interrupt] SIGKILL to process group -$PID" kill -KILL -- -"$PID" 2>/dev/null || kill -KILL "$PID" 2>/dev/null || true fi @@ -98,9 +92,9 @@ fi wait "$PID" 2>/dev/null rc=$? -# Only after the reap: an unreaped leader is still a member of its own group, so -# polling the group before `wait` would report it alive forever. Do not let the -# probe start while an installer process is still running. +# Only after the reap: an unreaped leader is still a member of its own group, so polling +# the group before `wait` would report it alive forever. The probe must not start while +# an installer process is still running. if [ "$killed" = "true" ]; then for _ in $(seq 1 "$KILL_GRACE"); do kill -0 -- -"$PID" 2>/dev/null || break @@ -115,8 +109,8 @@ echo "[interrupt] installer exit=$rc reason=$reason killed=$killed" echo "[interrupt] last log lines:" tail -15 "$LOG" || true -# Report how far it got, so a leg that never reached the target step is visible as such -# rather than passing for the wrong reason. +# Report how far it got, so a leg that never reached the target step is visible rather +# than passing for the wrong reason. if [ -n "$MARKER" ] && ! grep -qE "$MARKER" "$LOG" 2>/dev/null; then echo "::warning::marker '$MARKER' never appeared -- this leg killed at the deadline, not at the intended step" fi diff --git a/.github/scripts/interrupted_install_probe.py b/.github/scripts/interrupted_install_probe.py index 7699ade872..c85d0397dc 100644 --- a/.github/scripts/interrupted_install_probe.py +++ b/.github/scripts/interrupted_install_probe.py @@ -95,15 +95,11 @@ def main(argv: list[str]) -> int: print(f"[probe] {k:28} = {v}") # ── the two probes Tauri preflight actually runs ───────────────────────── - # Both under the DESKTOP's deadline, not a generous CI one. preflight wraps each - # call in a 10 second tokio timeout (managed.rs:337 for `-h`, managed.rs:390 for - # desktop-capabilities) and on expiry kills the child and reports Stale -- - # "cli_unusable" or "desktop_capability_probe_failed" (managed.rs:471, :521). A - # torn venv whose CLI still answers, but only after 30 seconds of import retries, - # is therefore an install the app sends to repair; waiting three minutes for it - # here would call the same install HEALTHY and skip the re-run assertion. run() - # reports a timeout as a non-zero rc, which lands in the same REPAIRABLE arm the - # desktop's Stale maps to. + # The DESKTOP's deadline, not a generous CI one: preflight times each call out after + # 10s (managed.rs:337 for `-h`, :390 for desktop-capabilities) and reports Stale + # (managed.rs:471, :521). A longer timeout here would call a slow torn venv HEALTHY + # and skip the re-run assertion. run() reports a timeout as a non-zero rc, landing in + # the same REPAIRABLE arm as Stale. PREFLIGHT_TIMEOUT = 10 t0 = time.time() @@ -123,18 +119,13 @@ def main(argv: list[str]) -> int: say("capabilities_ok", caps_rc == 0) say("capabilities_seconds", round(time.time() - t0, 2)) - # Parse EXACTLY as the desktop does: managed.rs:414 hands the whole stdout buffer - # to serde_json, which rejects any leading or trailing non-JSON, and stderr was - # already discarded at managed.rs:358. Folding stderr in and then scanning to the - # first brace made one warning line on stderr enough for json.loads to raise on - # the trailing text, leaving studio_install_ok "absent" and reporting FALSE_READY - # over an install the real app parses, sees as incomplete, and offers to repair. - # - # studio_install_ok is added by the install-manifest work, so it is absent on older - # trees; that is recorded separately from present-and-false only to make the - # artefact readable, because the desktop treats both as Stale. A payload that does - # not parse at all is a third case with the same outcome: the desktop gets None - # back and reports Stale ("desktop_capability_probe_failed", managed.rs:521). + # Parse EXACTLY as the desktop does: managed.rs:414 hands the whole stdout buffer to + # serde_json, which rejects leading or trailing non-JSON, and stderr was already + # discarded at managed.rs:358. Folding stderr in made one warning line enough to fail + # the parse and report FALSE_READY over an install the real app offers to repair. + # "absent" (studio_install_ok predates the install-manifest work) and "unparseable" + # are split apart only for a readable artefact: the desktop reports Stale for both + # ("desktop_capability_probe_failed", managed.rs:521). install_ok: object = "absent" try: parsed = json.loads(caps_out) @@ -147,26 +138,21 @@ def main(argv: list[str]) -> int: install_ok = "unparseable" say("capabilities.studio_install_ok", install_ok) - # The desktop's own conclusion: Ready only when the probe exits 0, the payload - # parses, AND studio_install_ok is true. The predicate is `!= Some(true)` - # (managed.rs:445), so an ABSENT field is Stale exactly like a false one -- a CLI - # too old to answer is already rejected one check earlier on - # desktop_manageability_version. Leaving "absent" undecided judged those installs - # on the backend alone, so a payload that stopped carrying the field reported - # HEALTHY on every booting leg and skipped the repair assertion this workflow - # exists to make, while the real app showed Stale and offered repair. That is the - # regression `unsloth_cli/commands/studio.py` is in this workflow's path filter to - # catch, so it must never be the thing that silences it. + # The desktop's own conclusion: Ready only on rc 0 + a parsed payload + a true + # studio_install_ok. The predicate is `!= Some(true)` (managed.rs:445), so an ABSENT + # field is Stale exactly like a false one; a CLI too old to answer is already + # rejected one check earlier on desktop_manageability_version. Leaving "absent" + # undecided reported HEALTHY on every booting leg and skipped the repair assertion + # this workflow exists to make -- the regression `unsloth_cli/commands/studio.py` + # sits in the path filter to catch, so it must never be what silences it. caps_ready = caps_rc == 0 and install_ok is True say("desktop_would_call_install_ok", caps_ready) # ── the deeper probes the fix PRs add ──────────────────────────────────── - # RECORDED, but NOT repair evidence: preflight/managed.rs runs only `-h` and - # `studio desktop-capabilities --json` (managed.rs:357) and reads - # studio_install_ok from that payload (managed.rs:445). It never invokes these - # two commands, so counting them would let a leg pass while the real app still - # reports ManagedReady over a torn install -- the exact false negative this - # workflow exists to catch. + # RECORDED, but NOT repair evidence: preflight runs only `-h` and + # `studio desktop-capabilities --json` (managed.rs:357, :445) and never these two, so + # counting them would let a leg pass while the real app still reports ManagedReady + # over a torn install -- the false negative this workflow exists to catch. for label, args in ( ("verify_install", ["studio", "verify-install"]), ("desktop_runtime_check", ["studio", "desktop-runtime-check"]), @@ -178,35 +164,32 @@ def main(argv: list[str]) -> int: (out / f"{label}.log").write_text(merged(r), encoding = "utf-8", errors = "replace") say(label, "ok" if r[0] == 0 else "failed") - # The in-progress marker #7490 writes before spawning the installer. RECORDED ONLY, - # never used as repair evidence: both interrupt drivers seed it before every install - # and deliberately never clear it, so it is true on every leg by construction. Using - # it in the verdict below would make REPAIRABLE unconditional and FALSE_READY -- the - # single outcome this workflow exists to catch -- unreachable. + # The in-progress marker #7490 writes before spawning the installer. RECORDED ONLY: + # both interrupt drivers seed it and deliberately never clear it, so it is true on + # every leg by construction, and using it in the verdict below would make REPAIRABLE + # unconditional and FALSE_READY -- the one outcome this catches -- unreachable. home = Path(os.environ.get("UNSLOTH_STUDIO_HOME") or (Path.home() / ".unsloth" / "studio")) say("install_in_progress_marker", (home / ".desktop-install-in-progress").exists()) # ── ground truth: does the backend actually boot? ──────────────────────── - # Own the whole process tree: the CLI spawns uvicorn/python children, and - # terminating only the parent leaves them holding the port, so the next leg's - # probe would hang. Same reason the interrupt driver kills the group. + # Own the whole process tree: the CLI spawns uvicorn/python children that would keep + # holding the port and hang the next leg's probe. Same reason the driver kills the + # group. popen_kw: dict = {} if os.name == "posix": popen_kw["start_new_session"] = True else: popen_kw["creationflags"] = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) - # Straight to the artefact file, never a PIPE: nothing reads that pipe until after - # the polling loop, so a backend whose imports emit more than the OS pipe buffer - # (64 KiB on Linux and macOS, a single page by default on Windows) blocks on write - # BEFORE it binds the port. backend_ok, which this whole verdict pivots on, would - # then be false for a perfectly good install. + # Straight to the artefact file, never a PIPE: nothing drains a pipe until after the + # polling loop, so a backend whose imports outrun the OS buffer (64 KiB on Linux and + # macOS, one page on Windows) blocks on write BEFORE binding the port, and backend_ok + # -- what this verdict pivots on -- would be false for a perfectly good install. blog_path = out / "backend.log" blog_fh = blog_path.open("w", encoding = "utf-8", errors = "replace") - # An interrupted install can leave the console script in place while its venv - # interpreter is gone: the earlier probes then report failure through run()'s - # OSError catch, but an unguarded spawn here raises instead, so no verdict.json - # is written and both workflows die on the json.load rather than reporting. An - # unlaunchable CLI is a broken backend that `-h` already flags -> REPAIRABLE. + # An interrupted install can leave the console script in place with its venv + # interpreter gone. run() catches that as OSError, but an unguarded spawn here would + # raise, so no verdict.json is written and both workflows die on the json.load rather + # than reporting. An unlaunchable CLI is a broken backend `-h` already flags. proc = None try: proc = subprocess.Popen( @@ -241,9 +224,9 @@ def main(argv: list[str]) -> int: if os.name == "posix": import signal - # start_new_session made this child its own group leader, so pgid == pid. - # Read it BEFORE the reap: once the leader is waited on, os.getpgid() - # raises and the escalation would target nothing. + # start_new_session made this child its own group leader. Read the pgid + # BEFORE the reap: once the leader is waited on, os.getpgid() raises and the + # escalation would target nothing. try: pgid = os.getpgid(proc.pid) except OSError: @@ -259,9 +242,8 @@ def main(argv: list[str]) -> int: except subprocess.TimeoutExpired: continue # Unconditional, and to the GROUP -- the same escalation - # interrupt-install.sh:94 makes, for the same reason. The leader exits - # promptly on SIGTERM while a uvicorn worker does not, so returning as - # soon as proc.wait() succeeded skipped the SIGKILL entirely and left + # interrupt-install.sh:94 makes. The leader exits promptly on SIGTERM while a + # uvicorn worker does not, so returning as soon as proc.wait() succeeded left # that worker holding the port and the venv open while the repair step # reinstalled underneath it. Signalling an empty group is a no-op. try: @@ -270,10 +252,10 @@ def main(argv: list[str]) -> int: pass else: # On win32 the CLI re-spawns the server as a CHILD and waits on it - # (unsloth_cli/commands/studio.py:1543); CREATE_NEW_PROCESS_GROUP does not + # (unsloth_cli/commands/studio.py:1543), and CREATE_NEW_PROCESS_GROUP does not # make terminate() reach descendants, so killing the wrapper alone leaves a - # server holding the venv open and the repair step reinstalls into files - # Windows has locked. taskkill /T takes the tree. + # server holding the venv open and the repair reinstalls into locked files. + # taskkill /T takes the tree. run(["taskkill", "/F", "/T", "/PID", str(proc.pid)], timeout = 30) try: proc.wait(timeout = 10) @@ -297,19 +279,16 @@ def main(argv: list[str]) -> int: say("backend_error", missing) # ── verdict ────────────────────────────────────────────────────────────── - # A booting backend is not enough to call the install finished. The manifest is - # written LAST (install_python_stack.py:3255), so a kill after "studio deps" but - # before it -- the data-designer leg -- leaves a venv whose backend boots while - # desktop-capabilities still says studio_install_ok=false, and preflight reports - # Stale (managed.rs:445) rather than Ready. Calling that HEALTHY skipped the - # re-run step, so the leg asserted nothing beyond a marker appearing and never - # exercised the fast path that is supposed to clear an incomplete install. + # A booting backend is not enough. The manifest is written LAST + # (install_python_stack.py:3255), so the data-designer leg boots while + # desktop-capabilities still says studio_install_ok=false and preflight reports Stale + # (managed.rs:445). Calling that HEALTHY skipped the re-run step, leaving the leg + # asserting nothing beyond a marker appearing. # - # `-h` gates the whole thing for the same reason: probe_managed_bin runs it FIRST - # and returns Stale "cli_unusable" without ever reaching the capability probe - # (managed.rs:465-478). Consulting cli_h_ok only in the repairable arm below let a - # CLI that cannot even print help be called HEALTHY as long as the backend booted, - # which skipped the re-run step for an install the app itself sends to repair. + # `-h` gates it for the same reason: probe_managed_bin runs it FIRST and returns + # Stale "cli_unusable" without reaching the capability probe (managed.rs:465-478), so + # consulting cli_h_ok only in the repairable arm called a CLI that cannot print help + # HEALTHY whenever the backend booted. if backend_ok and caps_ready and facts.get("cli_h_ok"): verdict = "HEALTHY" elif not caps_ready or not facts.get("cli_h_ok"): diff --git a/.github/workflows/interrupted-install-ci.yml b/.github/workflows/interrupted-install-ci.yml index f73094b89a..6c3c3b58f5 100644 --- a/.github/workflows/interrupted-install-ci.yml +++ b/.github/workflows/interrupted-install-ci.yml @@ -3,19 +3,17 @@ # Proves an INTERRUPTED install can never masquerade as a healthy one. # -# Reported failure: a user quits the desktop app while it is installing. The app kills -# the installer process group (main.rs cleanup_child_processes -> install.rs:798-807), -# which lands mid "studio deps" -- the step that installs -# studio/backend/requirements/studio.txt, where structlog is declared. On relaunch, -# preflight probes `unsloth -h` and `studio desktop-capabilities --json`; both succeed -# because the CLI's own deps (typer/click/rich) are core, so the app reports -# ManagedReady with can_auto_repair=false. The backend then dies on -# `import structlog` and the user is permanently stuck on "Server stopped -# unexpectedly". +# Reported failure: quitting the app mid-install kills the installer process group +# (main.rs cleanup_child_processes -> install.rs:798-807), landing mid "studio deps" -- +# the step installing studio/backend/requirements/studio.txt, where structlog is +# declared. On relaunch preflight probes `unsloth -h` and `studio desktop-capabilities +# --json`; both succeed because the CLI's own deps (typer/click/rich) are core, so the +# app reports ManagedReady with can_auto_repair=false while the backend dies on +# `import structlog` and the user is stuck on "Server stopped unexpectedly". # -# Nothing in CI covered this: no job has ever interrupted an install. This workflow -# kills the installer at each interesting phase and asserts the result is either -# genuinely healthy or explicitly repairable -- never silently ready. +# No CI job had ever interrupted an install. This one kills the installer at each phase +# and asserts the result is genuinely healthy or explicitly repairable, never silently +# ready. name: Interrupted install recovery @@ -32,27 +30,23 @@ on: - 'studio/src-tauri/src/preflight/**' - 'unsloth_cli/commands/studio.py' # Every leg installs the checkout with `--local`, so this file decides the - # `unsloth` console script and the core dependencies the probe leans on: - # `-h` and `desktop-capabilities` only survive a torn install because - # typer/click/rich are declared here. Moving one of those to an extra - # changes what every interrupted venv looks like, and no other install - # workflow interrupts the installer, so such a PR would otherwise merge - # without a single leg running. + # `unsloth` console script and the core deps the probe leans on: `-h` and + # `desktop-capabilities` only survive a torn install because typer/click/rich + # are declared here. Moving one to an extra changes what every interrupted + # venv looks like, and no other install workflow interrupts the installer. - 'pyproject.toml' # studio_install_ok and verify-install, the two decisions the probe asserts - # on, are implemented here rather than in commands/studio.py, so a change - # that made install_state() accept a missing manifest would otherwise merge - # without a single leg running. + # on, live here rather than in commands/studio.py, so a change making + # install_state() accept a missing manifest would otherwise merge unrun. - 'unsloth_cli/_studio_deps.py' - 'studio/install_manifest.py' - # The requirement files are the phases. studio.txt is where structlog is - # declared, the package whose absence IS the reported false-ready bug, and - # the single-env files drive the later steps, so moving a package between - # them changes what every interrupted state looks like. + # The requirement files are the phases: studio.txt declares structlog, whose + # absence IS the reported false-ready bug, so moving a package between them + # changes what every interrupted state looks like. - 'studio/backend/requirements/**' - # `*` never matches `/`, and it is a literal `-install` that follows, so - # `interrupt*-install*` matches interrupt-install.sh / .ps1 but NOT the - # underscored probe. List the probe explicitly rather than rely on a glob. + # `*` never matches `/` and a literal `-install` follows, so + # `interrupt*-install*` would match the .sh / .ps1 but NOT the underscored + # probe. List all three explicitly rather than rely on a glob. - '.github/scripts/interrupt-install.sh' - '.github/scripts/interrupt-install.ps1' - '.github/scripts/interrupted_install_probe.py' @@ -89,10 +83,9 @@ jobs: - {os: macos-14, label: setup, marker: '\[TAURI:STEP\] Running Unsloth setup', experimental: false} # Other dependency-pass steps around the named one. - {os: macos-14, label: pip-bootstrap, marker: 'pip bootstrap', experimental: false} - # No base-packages cell: install.sh --local sets skip_base, so - # install_python_stack returns before any "base packages" label is - # printed and the kill can never land. It ran to completion instead, - # proving nothing. + # No base-packages cell: --local sets skip_base, so install_python_stack + # returns before any "base packages" label prints and the kill can never + # land. That leg ran to completion instead, proving nothing. - {os: macos-14, label: unsloth-extras, marker: 'unsloth extras', experimental: true} - {os: macos-14, label: data-designer, marker: 'data designer deps', experimental: true} # Linux: same teardown path, different package manager and process semantics. @@ -115,14 +108,13 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} KILL_AT_SECONDS: '1500' run: | - # --local is load-bearing, not a convenience. Without it install.sh:3996 - # resolves `unsloth>=2026.7.5` from PyPI, so the venv gets the PUBLISHED CLI - # and none of the branch's unsloth_cli changes are present. Every probe of - # `studio verify-install` / `studio desktop-runtime-check` then reports - # "absent" no matter what the branch does, which makes the whole lane - # incapable of observing the fix it exists to test. --local overlays the - # checkout editable (install.sh:3990) before `studio setup` runs the dep - # pass, so a kill at "studio deps" leaves the branch's CLI installed. + # --local is load-bearing. Without it install.sh:3996 resolves + # `unsloth>=2026.7.5` from PyPI, so the venv gets the PUBLISHED CLI, every + # probe of `studio verify-install` / `desktop-runtime-check` reports "absent" + # whatever the branch does, and the lane cannot observe the fix it tests. + # --local overlays the checkout editable (install.sh:3990) before `studio + # setup` runs the dep pass, so a kill at "studio deps" leaves the branch's + # CLI installed. bash .github/scripts/interrupt-install.sh \ '${{ matrix.marker }}' logs/install.log -- --tauri --local @@ -146,8 +138,8 @@ jobs: BIN="$HOME/.unsloth/studio/unsloth_studio/bin/unsloth" [ -x "$BIN" ] || BIN="$HOME/.unsloth/studio/bin/unsloth" if [ ! -x "$BIN" ]; then - # No CLI at all is a SAFE outcome: preflight reports NotInstalled and the - # app offers a normal install. Nothing to assert beyond that. + # No CLI at all is SAFE: preflight reports NotInstalled and the app offers + # a normal install. echo "verdict=NO_CLI" >> "$GITHUB_OUTPUT" echo "[probe] no unsloth CLI installed -> preflight reports NotInstalled (safe)" exit 0 @@ -159,16 +151,16 @@ jobs: exit "$rc" - name: A re-run must repair, not short-circuit - # NO_CLI included. A kill at venv or torch lands before "Installing Unsloth" - # (install.sh:2125 / :3667 / :3961), so those legs always take NO_CLI, and - # skipping the re-run left three non-experimental legs asserting nothing but - # that a marker appeared. The bug's second half is that `install.sh` sees a - # "current" version and no-ops over a broken venv. + # NO_CLI included: a kill at venv or torch lands before "Installing Unsloth" + # (install.sh:2125 / :3667 / :3961), so those legs always take NO_CLI and + # skipping the re-run left three non-experimental legs asserting only that a + # marker appeared. The bug's second half is `install.sh` seeing a "current" + # version and no-opping over a broken venv. # - # HEALTHY means the backend boots AND desktop-capabilities reports the - # install complete, so a leg killed after "studio deps" but before the - # manifest is written last (install_python_stack.py:3255) -- data-designer -- - # arrives here rather than skipping the one assertion that matters for it. + # HEALTHY needs the backend booting AND the install reported complete, so the + # data-designer leg (killed after "studio deps" but before the manifest is + # written last, install_python_stack.py:3255) arrives here rather than skipping + # the one assertion that matters for it. if: always() && steps.probe.outputs.verdict != 'HEALTHY' run: | set -o pipefail @@ -177,8 +169,8 @@ jobs: echo "repair exit: $rc" BIN="$HOME/.unsloth/studio/unsloth_studio/bin/unsloth" [ -x "$BIN" ] || BIN="$HOME/.unsloth/studio/bin/unsloth" - # The probe exits without writing verdict.json when the bin is missing, so - # check here or the json.load below crashes instead of reporting. + # The probe writes no verdict.json when the bin is missing, so check here or + # the json.load below crashes instead of reporting. if [ ! -x "$BIN" ]; then echo "::error::after a full re-run there is still no unsloth CLI at $BIN" tail -30 logs/repair.log || true @@ -186,9 +178,9 @@ jobs: fi python3 .github/scripts/interrupted_install_probe.py "$BIN" --out probe-after || true v="$(python3 -c "import json;print(json.load(open('probe-after/verdict.json'))['verdict'])")" - # A booting backend IS the repair, whatever the log narrated. Judging by - # log text instead failed a leg whose venv was fine: the only match was - # the frontend build printing "up to date". + # A booting backend IS the repair, whatever the log narrated. Judging by log + # text failed a leg whose venv was fine: the only match was the frontend + # build printing "up to date". if [ "$v" = "HEALTHY" ]; then echo "re-run repaired the install (verdict=HEALTHY)" exit 0 @@ -214,23 +206,22 @@ jobs: # ── Windows: no process groups, so the kill path differs ────────────────── interrupt-windows: name: windows kill@${{ matrix.label }} - # No UNSLOTH_STUDIO_HOME here. The desktop app always invokes the installer as - # `--tauri [--local]` and scrubs the variable first (install.rs:202 and :356), and - # install.ps1:189-215 rejects a custom root under --tauri, so a workspace-scoped - # root forced these legs down the shell-install path instead: UNSLOTH_TAURI_MODE=0, - # frontend build enabled, different root resolution, no bundled-file overlay. Worse, - # "Installing PyTorch" is only ever printed by Write-TauriLog (install.ps1:2440), so - # without --tauri the torch leg's marker could not appear at all. The runner is - # ephemeral, so the default root is safe to install into. + # No UNSLOTH_STUDIO_HOME here. The app scrubs it before invoking the installer as + # `--tauri [--local]` (install.rs:202, :356) and install.ps1:189-215 rejects a custom + # root under --tauri, so a workspace-scoped root forced these legs down the + # shell-install path: UNSLOTH_TAURI_MODE=0, frontend build on, different root + # resolution, no bundled-file overlay. Worse, "Installing PyTorch" is only printed by + # Write-TauriLog (install.ps1:2440), so the torch leg's marker could never appear. + # The runner is ephemeral, so the default root is safe to install into. runs-on: windows-latest timeout-minutes: 60 strategy: fail-fast: false matrix: include: - # install.ps1 parses `--no-torch` (install.ps1:121); `-SkipTorch` matches no - # case in that switch and is silently dropped. The torch leg must NOT skip - # torch, or its marker never appears. + # install.ps1:121 parses `--no-torch`; `-SkipTorch` matches no case there and + # is silently dropped. The torch leg must NOT skip torch, or its marker never + # appears. - {label: studio-deps, marker: 'studio deps', installArgs: '--tauri --no-torch --local'} - {label: torch, marker: 'Installing PyTorch', installArgs: '--tauri --local'} @@ -274,11 +265,10 @@ jobs: Write-Host '[probe] no unsloth CLI -> preflight reports NotInstalled (safe)' exit 0 } - # The SAME probe the other platforms run. This step used to be a bespoke - # inline version that only checked `-h` and `desktop-capabilities`, so it - # could not observe studio_install_ok / verify-install / - # desktop-runtime-check -- it would have failed the very PRs that add them, - # no matter how well they worked. + # The SAME probe the other platforms run. The bespoke inline version this + # replaced only checked `-h` and `desktop-capabilities`, so it could not + # observe studio_install_ok / verify-install / desktop-runtime-check and + # would have failed the very PRs that add them. python .github/scripts/interrupted_install_probe.py $bin --out probe $rc = $LASTEXITCODE $v = (Get-Content probe/verdict.json -Raw | ConvertFrom-Json).verdict @@ -286,17 +276,16 @@ jobs: exit $rc - name: A re-run must repair, not short-circuit - # Same assertion the POSIX legs make, NO_CLI included: a leg that left no CLI - # otherwise asserts nothing, and without this a Windows leg proves only that - # the break was DETECTED, never that install.ps1's version fast path does not - # short-circuit over it, which is the half of the bug that strands the user. + # Same assertion the POSIX legs make, NO_CLI included: without it a Windows leg + # proves only that the break was DETECTED, never that install.ps1's version fast + # path does not short-circuit over it -- the half of the bug that strands the + # user. if: always() && steps.probe.outputs.verdict != 'HEALTHY' shell: pwsh run: | - # powershell.exe with install.rs:325-339's flags, matching the interrupted - # run: the desktop repairs by re-running the same bundled script in Windows - # PowerShell 5.1, so a repair that only works under pwsh 7 would pass here - # and still strand the user. + # powershell.exe with install.rs:325-339's flags, matching the interrupted run: + # the desktop repairs under Windows PowerShell 5.1, so a repair that only works + # under pwsh 7 would pass here and still strand the user. powershell.exe -NoLogo -NoProfile -NonInteractive -WindowStyle Hidden ` -ExecutionPolicy Bypass -File install.ps1 ${{ matrix.installArgs }} *>&1 | Tee-Object -FilePath logs/repair.log @@ -308,9 +297,9 @@ jobs: } python .github/scripts/interrupted_install_probe.py $bin --out probe-after $v = (Get-Content probe-after/verdict.json -Raw | ConvertFrom-Json).verdict - # A booting backend IS the repair, whatever the log narrated. Judging by - # log text instead failed a POSIX leg whose venv was fine: the only match - # was the frontend build printing "up to date". + # A booting backend IS the repair, whatever the log narrated. Judging by log + # text failed a POSIX leg whose venv was fine: the only match was the frontend + # build printing "up to date". if ($v -eq 'HEALTHY') { Write-Host 're-run repaired the install (verdict=HEALTHY)' exit 0