Address review feedback: reroute flags, curl EPIPE, post-install browser open
Three fixes from PR review and field testing: - Forward an explicit --no-browser/--browser choice into the WSL Strix Halo reroute so the rerouted install honors the flag. - Drain piped stdin before the --shortcuts-only early exit so curl | sh -s -- --shortcuts-only no longer dies with curl error 23. - Open the browser after the installer's own foreground launch when the preference is on: a background watcher polls /api/health, verifies the per-install studio_root_id so a different Studio on the port is never opened, then opens the URL once. Mirrored in install.ps1 with a Start-Job watcher. When the preference is off nothing changes; the server already prints its URL.
This commit is contained in:
Unsloth
parent
b03142eac8
commit
86200cb0be
3 changed files with 111 additions and 5 deletions
31
install.ps1
31
install.ps1
|
|
@ -2671,10 +2671,41 @@ exit 0
|
|||
# In interactive terminals, ask the user before starting Studio.
|
||||
# In non-interactive environments (CI, Docker) just print instructions.
|
||||
$IsInteractive = [Environment]::UserInteractive -and (-not [Console]::IsInputRedirected)
|
||||
# Background watcher for the foreground launch below: once the server is
|
||||
# healthy, open the browser per the persisted preference (mirrors the
|
||||
# desktop launcher). Guarded by the per-install root id so a different
|
||||
# Studio already on the port is never the one opened.
|
||||
$_browserWatch = {
|
||||
param($RootId, $Port)
|
||||
$deadline = (Get-Date).AddSeconds(120)
|
||||
while ((Get-Date) -lt $deadline) {
|
||||
try {
|
||||
$r = Invoke-RestMethod -Uri "http://127.0.0.1:$Port/api/health" -TimeoutSec 1 -Method Get
|
||||
if ($r.service -eq 'Unsloth UI Backend' -and
|
||||
((-not $RootId) -or $r.studio_root_id -eq $RootId)) {
|
||||
Start-Process "http://localhost:$Port"
|
||||
break
|
||||
}
|
||||
} catch {}
|
||||
Start-Sleep -Seconds 1
|
||||
}
|
||||
}
|
||||
if ($IsInteractive) {
|
||||
Write-Host ""
|
||||
$reply = Read-Host " Start Unsloth Studio now? [Y/n]"
|
||||
if ([string]::IsNullOrWhiteSpace($reply) -or $reply -match '^[Yy]') {
|
||||
# Open the browser once the server is up, unless opted out. The
|
||||
# server prints its own URL, so no watcher is needed when off.
|
||||
if ($OpenBrowserPref -ne '0') {
|
||||
$_watchRootId = ""
|
||||
$_watchIdFile = Join-Path $StudioHome "share\studio_install_id"
|
||||
if (Test-Path -LiteralPath $_watchIdFile) {
|
||||
try { $_watchRootId = ([System.IO.File]::ReadAllText($_watchIdFile)).Trim() } catch {}
|
||||
}
|
||||
try {
|
||||
$null = Start-Job -ScriptBlock $_browserWatch -ArgumentList @($_watchRootId, 8888)
|
||||
} catch {}
|
||||
}
|
||||
& $UnslothExe studio -p 8888
|
||||
} else {
|
||||
step "launch" "to start later, run:"
|
||||
|
|
|
|||
56
install.sh
56
install.sh
|
|
@ -1433,6 +1433,10 @@ if [ "$_SHORTCUTS_ONLY" = true ]; then
|
|||
fi
|
||||
create_studio_shortcuts "$VENV_ABS_BIN/unsloth" "$OS"
|
||||
fi
|
||||
# Drain piped stdin (curl | sh -s -- --shortcuts-only ...) before this
|
||||
# early exit; otherwise curl dies with EPIPE, prints "curl: (23) Failure
|
||||
# writing output to destination", and fails the whole pipeline.
|
||||
[ ! -t 0 ] && cat > /dev/null 2>&1
|
||||
exit 0
|
||||
fi
|
||||
|
||||
|
|
@ -1665,6 +1669,10 @@ _maybe_reroute_strixhalo_to_2404() {
|
|||
[ -n "$_USER_PYTHON" ] && _rr_args="$_rr_args --python $(_rr_q "$_USER_PYTHON")"
|
||||
[ "$_VERBOSE" = true ] && _rr_args="$_rr_args --verbose"
|
||||
[ "$TAURI_MODE" = true ] && _rr_args="$_rr_args --tauri"
|
||||
# Forward an explicit browser choice; "" (undecided) forwards nothing so
|
||||
# the rerouted install keeps its own default.
|
||||
[ "$_STUDIO_OPEN_BROWSER" = "0" ] && _rr_args="$_rr_args --no-browser"
|
||||
[ "$_STUDIO_OPEN_BROWSER" = "1" ] && _rr_args="$_rr_args --browser"
|
||||
if [ -n "${UNSLOTH_WSL_REROUTE_CMD:-}" ]; then
|
||||
_rr_cmd="$UNSLOTH_WSL_REROUTE_CMD" # user took full control
|
||||
elif [ -n "$_rr_args" ]; then
|
||||
|
|
@ -3277,6 +3285,49 @@ printf " ${C_TITLE}%s${C_RST}\n" "Unsloth Studio installed!"
|
|||
printf " ${C_DIM}%s${C_RST}\n" "$RULE"
|
||||
echo ""
|
||||
|
||||
# Background watcher for the post-install foreground launch below: once the
|
||||
# server is healthy, open the browser per the persisted preference (mirrors
|
||||
# the desktop launcher). Guarded by the per-install root id so a different
|
||||
# Studio already on the port is never the one opened.
|
||||
_post_install_browser_watch() {
|
||||
_pibw_port="$1"
|
||||
_pibw_url="http://localhost:$_pibw_port"
|
||||
_pibw_id=$(cat "$STUDIO_HOME/share/studio_install_id" 2>/dev/null || true)
|
||||
(
|
||||
_pibw_deadline=$(($(date +%s) + 120))
|
||||
while [ "$(date +%s)" -lt "$_pibw_deadline" ]; do
|
||||
_pibw_resp=$(curl -fsS --max-time 1 "http://127.0.0.1:$_pibw_port/api/health" 2>/dev/null \
|
||||
|| wget -qO- --timeout=1 "http://127.0.0.1:$_pibw_port/api/health" 2>/dev/null \
|
||||
|| true)
|
||||
case "$_pibw_resp" in
|
||||
*'"Unsloth UI Backend"'*)
|
||||
if [ -n "$_pibw_id" ]; then
|
||||
case "$_pibw_resp" in
|
||||
*"\"studio_root_id\":\"$_pibw_id\""*|*"\"studio_root_id\": \"$_pibw_id\""*) ;;
|
||||
*) sleep 1; continue ;;
|
||||
esac
|
||||
fi
|
||||
if [ "$(uname)" = "Darwin" ] && command -v open >/dev/null 2>&1; then
|
||||
open "$_pibw_url" 2>/dev/null
|
||||
elif grep -qi microsoft /proc/version 2>/dev/null; then
|
||||
if command -v powershell.exe >/dev/null 2>&1; then
|
||||
powershell.exe -NoProfile -Command "Start-Process '$_pibw_url'" >/dev/null 2>&1
|
||||
elif command -v cmd.exe >/dev/null 2>&1; then
|
||||
cmd.exe /c start "" "$_pibw_url" >/dev/null 2>&1
|
||||
elif command -v xdg-open >/dev/null 2>&1; then
|
||||
xdg-open "$_pibw_url" >/dev/null 2>&1
|
||||
fi
|
||||
elif command -v xdg-open >/dev/null 2>&1; then
|
||||
xdg-open "$_pibw_url" >/dev/null 2>&1
|
||||
fi
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
sleep 1
|
||||
done
|
||||
) &
|
||||
}
|
||||
|
||||
# In interactive terminals, ask the user before starting Studio.
|
||||
# In non-interactive environments (Docker, CI, cloud-init) just print instructions.
|
||||
if [ -t 1 ]; then
|
||||
|
|
@ -3291,6 +3342,11 @@ if [ -t 1 ]; then
|
|||
case "${_reply:-y}" in
|
||||
[Yy]*|"")
|
||||
step "launch" "starting Unsloth Studio..."
|
||||
# Open the browser once the server is up, unless opted out. The
|
||||
# server prints its own URL, so no watcher is needed when off.
|
||||
if [ "${_STUDIO_OPEN_BROWSER:-1}" != "0" ]; then
|
||||
_post_install_browser_watch 8888
|
||||
fi
|
||||
# Detach stdin from the `curl | sh` pipe: as a foreground server the
|
||||
# studio would otherwise drain the rest of this piped script, leaving
|
||||
# the shell to die parsing the now-truncated tail (`unexpected fi`).
|
||||
|
|
|
|||
|
|
@ -87,6 +87,18 @@ assert_contains \
|
|||
assert_contains \
|
||||
"install.sh: prompt Enter keeps the persisted preference" \
|
||||
"$_installer" '*) _STUDIO_OPEN_BROWSER="${_existing_open_browser:-1}"'
|
||||
# The WSL Strix Halo reroute must forward an explicit browser choice.
|
||||
assert_contains \
|
||||
"install.sh: reroute forwards --no-browser" \
|
||||
"$_installer" '[ "$_STUDIO_OPEN_BROWSER" = "0" ] && _rr_args="$_rr_args --no-browser"'
|
||||
# The post-install foreground launch honors the preference too.
|
||||
assert_contains \
|
||||
"install.sh: post-install launch opens browser via gated watcher" \
|
||||
"$_installer" "_post_install_browser_watch 8888"
|
||||
# The --shortcuts-only early exit must not EPIPE a curl | sh pipeline.
|
||||
assert_contains \
|
||||
"install.sh: shortcuts-only exit drains piped stdin" \
|
||||
"$_installer" '[ ! -t 0 ] && cat > /dev/null'
|
||||
|
||||
echo ""
|
||||
echo "=== install.sh _open_browser gating (functional) ==="
|
||||
|
|
@ -167,13 +179,20 @@ assert_file_contains \
|
|||
assert_file_contains \
|
||||
"install.ps1: prompt Enter keeps the baked preference" \
|
||||
"$INSTALL_PS1" 'elseif ($_existingPref) { $_existingPref }'
|
||||
# All launcher URL opens must route through the gated helper.
|
||||
_ps1_direct_open=$(grep -cF 'Start-Process "http://localhost:' "$INSTALL_PS1" || true)
|
||||
if [ "$_ps1_direct_open" -eq 0 ]; then
|
||||
echo " PASS: no ungated Start-Process http://localhost calls remain"
|
||||
assert_file_contains \
|
||||
"install.ps1: post-install launch opens browser via gated watcher" \
|
||||
"$INSTALL_PS1" 'Start-Job -ScriptBlock $_browserWatch'
|
||||
# All launcher URL opens must route through the gated helper. The one
|
||||
# allowed direct call is inside the post-install $_browserWatch scriptblock,
|
||||
# whose Start-Job call site is itself gated on the preference.
|
||||
_ps1_direct_open=$(grep -cE 'Start-Process "http://localhost:' "$INSTALL_PS1" || true)
|
||||
_ps1_watch_open=$(awk '/\$_browserWatch = \{/{f=1} f && /^ \}$/{exit} f' "$INSTALL_PS1" \
|
||||
| grep -cE 'Start-Process "http://localhost:' || true)
|
||||
if [ "$_ps1_direct_open" -eq 1 ] && [ "$_ps1_watch_open" -eq 1 ]; then
|
||||
echo " PASS: only the gated browser watcher opens a URL directly"
|
||||
PASS=$((PASS + 1))
|
||||
else
|
||||
echo " FAIL: $_ps1_direct_open ungated Start-Process http://localhost call(s) remain"
|
||||
echo " FAIL: found $_ps1_direct_open direct URL opens ($_ps1_watch_open in the watcher); all others must route through Open-StudioUrl"
|
||||
FAIL=$((FAIL + 1))
|
||||
fi
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue