From 8c335b8da69164b9aa09df9ea15a4cca7f05706e Mon Sep 17 00:00:00 2001 From: Michael Han <107991372+shimmyshimmer@users.noreply.github.com> Date: Mon, 18 May 2026 02:09:42 -0700 Subject: [PATCH] studio/install: fix mac desktop shortcut spawning and lifecycle (#5496) * studio/install: fix mac desktop shortcut spawning and lifecycle The macOS .app generated by install.sh ships a shell-shim wrapper that is unsigned and has no NSAppleEventsUsageDescription in its Info.plist, so AppleEvents from the bundle are denied by TCC. The launcher's `osascript ... tell application "Terminal" to do script ...` call silently fails and the script falls back to the headless nohup branch, where the user sees no Terminal window at all. Each click of the Desktop shortcut then leaks an unattached server (no PID file, no cleanup) and the launcher times out after 60s without ever opening a browser. Replace the AppleScript spawn with a `.command` file + `open -a Terminal`. Terminal handles `.command` natively through Launch Services, no AppleEvents permission required, works with unsigned bundles. The new design also decouples the studio server from the Terminal: - Server is started via nohup, detached from any TTY. Warm relaunches (server still alive) hit the existing fast path: the launcher's `_find_healthy_port` returns the running port and the browser opens in ~80ms with no Terminal involvement. - The `.command` file is a log viewer (`tail -F` of studio.log), not the server's parent. It also runs a watcher subshell that polls the server PID and kills `tail` when the server exits. This means clicking "Stop server" in the UI causes the Terminal window to drop to no-running-processes state, so the user can close the window without the "Do you want to terminate running processes" dialog. - A trap on HUP/INT/TERM/EXIT in the `.command` file sends SIGTERM (then SIGKILL at +0.5s) to the server PID, so closing the Terminal window also stops Studio. Best of both worlds: fast warm relaunch AND "close terminal == quit Studio". Also: - Drop POLL_INTERVAL_SEC from 1 to 0.25. With Python studio startup at ~2s, the 1s poll added up to 1s of slack between server-ready and browser-open. 0.25s tightens cold-launch latency at no meaningful CPU cost. - Refuse to install the `.app` bundle through a symlink. If a prior install (e.g. a --tauri build) left $HOME/Applications/Unsloth\\ Studio.app as a symlink, mkdir -p follows it and writes the new bundle contents through to the target. Detect and rm the symlink before mkdir -p. Test plan: - Existing studio-mac-update-smoke.yml CI runs install.sh end-to-end on macos-14 and asserts /api/health returns healthy. - Manual: click Desktop shortcut from cold state, Terminal opens with logs streaming, browser opens at ~2s. Re-click while Studio still running, browser opens in <200ms, no new Terminal. Click "Stop server" in the UI, Terminal closes cleanly with no prompt. Close Terminal via Cmd+W, server stops within 1s. * studio/install: trim verbose comments in _spawn_terminal * studio/install: harden trap quoting in generated .command The trap bodies in the .command file were written with broken quoting: trap "rm -f "$PID_FILE" 2>/dev/null" EXIT Shell parses this as three concatenated tokens ("rm -f " + unquoted $PID_FILE + " 2>/dev/null") then runs the trap. With paths that contain spaces, the unquoted expansion word-splits and the rm either no-ops or removes the wrong path. Default $HOME has no spaces so the bug is latent, but it should be space-safe. Switch both trap bodies to single-quoted form so $WATCHER_PID, $TAIL_PID, and $PID_FILE expand at signal time inside properly quoted positions. Shellcheck-clean on the generated .command. * studio/install: exec studio in nohup wrapper so PID is the server Without the explicit exec, `nohup sh -c "$_cmd"` runs `_cmd` as a child of the wrapper shell. Whether sh exec-optimizes that single command is shell-specific (macOS /bin/sh does, dash does, some bash configurations do not). When the optimization does not fire, `$!` records the wrapper PID rather than the studio PID, so: - the watcher in the generated .command monitors the wrapper, not the actual studio process; closing the Terminal can leave studio running if the wrapper exits first - SIGTERM from shutdown_studio goes to the wrapper rather than the server Force the replacement with exec so the recorded PID is always the studio process regardless of shell version. Flagged by both gemini-code-assist and codex in PR review; verified correct. * Fix orphan-on-spawn-failure, graceful kill, and nested symlink for PR #5496 Three issues found while testing the new macOS spawn path: 1. _spawn_terminal returned 0 even when 'open -a Terminal' failed, so the nohup'd server was left orphaned with no Terminal owner. Wrap the .command write + chmod + open chain in 'if {...}; then return 0; fi', and on failure SIGTERM the orphan (with a 3s grace) before falling through to the generic terminal-spawn fallback. 2. The generated .command sent SIGKILL only 0.5s after SIGTERM, shorter than studio/backend/run.py's _graceful_shutdown windows (5s inference + 5s export). Wait up to 12s for the server to exit on its own. 3. The .app symlink guard only checked the top-level path. If a prior corrupted install left Unsloth Studio.app/Contents (or its MacOS or Resources children) as a symlink, mkdir -p still wrote through them. Check all four bundle paths, and refuse to continue if the bundle path exists as a regular file. --------- Co-authored-by: Daniel Han --- install.sh | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/install.sh b/install.sh index 9046a9bdf6..c7852c7539 100755 --- a/install.sh +++ b/install.sh @@ -572,7 +572,7 @@ fi BASE_PORT=8888 MAX_PORT_OFFSET=20 TIMEOUT_SEC=60 -POLL_INTERVAL_SEC=1 +POLL_INTERVAL_SEC=0.25 LOG_FILE="$DATA_DIR/studio.log" # why: in env-override mode multiple installs share an OS user; namespace the # lock and remember our own healthy port so we never attach to an unrelated @@ -727,9 +727,65 @@ _spawn_terminal() { _cmd="$1" _os=$(uname) if [ "$_os" = "Darwin" ]; then - # Escape backslashes and double-quotes for AppleScript string - _cmd_escaped=$(printf '%s' "$_cmd" | sed 's/\\/\\\\/g; s/"/\\"/g') - osascript -e "tell application \"Terminal\" to do script \"$_cmd_escaped\"" >/dev/null 2>&1 && return 0 + # AppleEvents are TCC-denied from unsigned .app bundles; spawn + # Terminal via a .command file + Launch Services instead. Server + # is nohup'd so warm relaunches hit the fast-path; watcher + trap + # in the .command couple Terminal close <-> server shutdown. + # `exec` keeps the recorded PID equal to the studio process so + # signals reach studio directly rather than a wrapper shell. + nohup sh -c "exec $_cmd" >> "$LOG_FILE" 2>&1 & + _server_pid=$! + _pid_file="$DATA_DIR/studio-$_launch_port.pid" + printf '%d\n' "$_server_pid" > "$_pid_file" 2>/dev/null || true + + _cmd_file="$DATA_DIR/launch-terminal.command" + _logfile_q=$(printf '%s' "$LOG_FILE" | sed "s/'/'\\\\''/g") + _pidfile_q=$(printf '%s' "$_pid_file" | sed "s/'/'\\\\''/g") + if { + { + printf '#!/bin/bash\n' + printf "SERVER_PID=%s\n" "$_server_pid" + printf "PID_FILE='%s'\n" "$_pidfile_q" + # Wait up to 12s for graceful shutdown before SIGKILL. + printf 'shutdown_studio() {\n' + printf ' kill -TERM "$SERVER_PID" 2>/dev/null\n' + printf ' _i=0\n' + printf ' while kill -0 "$SERVER_PID" 2>/dev/null && [ "$_i" -lt 24 ]; do\n' + printf ' sleep 0.5\n' + printf ' _i=$((_i + 1))\n' + printf ' done\n' + printf ' kill -0 "$SERVER_PID" 2>/dev/null && kill -KILL "$SERVER_PID" 2>/dev/null\n' + printf ' rm -f "$PID_FILE" 2>/dev/null\n' + printf '}\n' + printf "tail -n 100 -F '%s' &\n" "$_logfile_q" + printf 'TAIL_PID=$!\n' + # Server gone -> kill tail so bash exits cleanly. + printf '(\n' + printf ' while kill -0 "$SERVER_PID" 2>/dev/null; do sleep 1; done\n' + printf ' kill "$TAIL_PID" 2>/dev/null\n' + printf ') &\n' + printf 'WATCHER_PID=$!\n' + printf "trap 'shutdown_studio; kill \"\$WATCHER_PID\" \"\$TAIL_PID\" 2>/dev/null; exit' HUP INT TERM\n" + printf "trap 'rm -f \"\$PID_FILE\" 2>/dev/null' EXIT\n" + printf 'wait "$TAIL_PID" 2>/dev/null\n' + } > "$_cmd_file" 2>/dev/null \ + && chmod +x "$_cmd_file" 2>/dev/null \ + && open -a Terminal "$_cmd_file" 2>/dev/null + }; then + # Foreground Terminal (Launch Services spawns us backgrounded). + osascript -e 'tell application "Terminal" to activate' >/dev/null 2>&1 || true + return 0 + fi + # .command/open failed: kill orphan, fall through to generic fallback. + kill -TERM "$_server_pid" 2>/dev/null || true + _i=0 + while kill -0 "$_server_pid" 2>/dev/null && [ "$_i" -lt 6 ]; do + sleep 0.5 + _i=$((_i + 1)) + done + kill -0 "$_server_pid" 2>/dev/null && kill -KILL "$_server_pid" 2>/dev/null || true + rm -f "$_pid_file" 2>/dev/null || true + echo "[WARN] Could not open Terminal; falling back to background launch" >&2 else for _term in gnome-terminal konsole xfce4-terminal mate-terminal lxterminal xterm; do if command -v "$_term" >/dev/null 2>&1; then @@ -1005,6 +1061,17 @@ DESKTOP_EOF _css_contents="$_css_app/Contents" _css_macos_dir="$_css_contents/MacOS" _css_res_dir="$_css_contents/Resources" + # Recreate bundle if root or any subpath is a symlink (mkdir -p follows them). + if [ -L "$_css_app" ] || [ -L "$_css_contents" ] \ + || [ -L "$_css_macos_dir" ] || [ -L "$_css_res_dir" ]; then + rm -rf "$_css_app" 2>/dev/null || { + echo "[ERROR] $_css_app contains a symlinked bundle path; remove manually and re-run install" >&2 + return 1 + } + elif [ -e "$_css_app" ] && [ ! -d "$_css_app" ]; then + echo "[ERROR] $_css_app exists but is not a directory; remove manually and re-run install" >&2 + return 1 + fi mkdir -p "$_css_macos_dir" "$_css_res_dir" # Info.plist