Skip to content

fix(launchers): Start and Stop work on macOS units - #264

Closed
ezoushen wants to merge 1 commit into
MiaAI-Lab:mainfrom
ezoushen:fix/macos-launchers
Closed

ezoushen wants to merge 1 commit into
MiaAI-Lab:mainfrom
ezoushen:fix/macos-launchers

Conversation

@ezoushen

@ezoushen ezoushen commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Start and Stop now work on macOS units. On a Mac unit every Start and Stop of a registered model ended at [sparkdash] refusing: <dir> is world-writable, even for a 755 directory, so a launcher could not be used there at all. Linux units run the same commands as before, and the safety checks still fail closed.

Why

The shell sparkDash runs over SSH (server/llmlaunch/commands.js) reads /proc/<pid>/stat, calls GNU stat -c and starts the script with setsid. macOS has none of the three. The failing check is wwrite, which treats a mode it cannot read as world-writable, so the unreadable stat -c result blocked every launch. Mac units are advertised as supported, and the launcher tests could not pass on a Mac.

How it works

  • Process start time and parent pid come from /proc where it exists and from ps elsewhere. ps runs with LC_ALL=C: its start time is a date printed in the session's language, and a pid file written under one locale has to match when read under another.
  • stat tries the GNU form first and falls back to BSD stat -f for mode, owner and size.
  • newsid starts the script as session leader with setsid, or with perl's POSIX::setsid where there is none (macOS ships perl).
  • alive() tests kill -0 $PID instead of [ -d /proc/$PID ], keeping the start-time check against pid reuse.

Evidence

launcher.test.js runs the generated programs with real bash. On a Mac, 14 of its 21 tests fail on current main; with this change all 24 pass on macOS (bash 3.2) and on Linux (bash 5, util-linux setsid). Three tests are new: the pid file records the real start time (also under LC_ALL=de_DE.UTF-8); a running script's log is cut back past its size cap (this exercises BSD stat -f %z); and Start and Stop refuse, with exit 126 and the right reason, when stat cannot report the mode, cannot report the owner, can report neither, or reports another owner. Each case isolates one check: making an unreadable mode fail open, or removing the owner check, makes the new test fail. Also run: npm run typecheck, npm run build, vitest, and the server suite, where the remaining failures are the ones current main already has on a Mac.

Driven end to end from a Linux host through the launcher API on two MacBooks: Start, status running, Stop, status stopped, port closed, with a launchd service and with a real MLX server lane.

Merge risk

This touches the code that decides whether a script may be executed, so here is the exact delta for the privilege-relevant parts:

  • Authorization (wwrite, check_script). Only how the mode and owner are read changes: GNU stat -c, then BSD stat -f (fmode, fowner). The decisions are the same: a script or directory that is world-writable, not owned by the user, outside the home or a symlink is refused, and so is one whose mode or owner cannot be read. Three of the four new refusal cases isolate one check each.
  • Session tracking (pstart, ppidof, alive, newsid). These decide whether a second Start launches another copy and whether status reads running; they never grant execution. alive() now tests kill -0 $PID instead of [ -d /proc/$PID ]: a pid that belongs to another user (reuse) answers EPERM and counts as not alive, which is the same outcome the start-time check gave before, and a legacy pid file without a start time is treated as before.
  • Process start. newsid runs the same script as the same user with the same arguments; on macOS it execs through perl -MPOSIX instead of setsid, and a missing perl makes the start fail rather than run unmanaged.

Linux behaviour is otherwise the same commands and the same results in the existing tests. macOS has no flock, so the existing "one start at a time" lock stays a no-op there; the manager's one-job-per-unit rule still applies.

Scope

In: the launcher shell and its tests, plus a CHANGELOG entry. Out: launchd lane scripts (a user script that runs launchctl), and api.test.js, which needs the local-Spark path and fails on macOS for an unrelated reason.

@MiaAI-Lab

Copy link
Copy Markdown
Owner

The macOS launcher failure is consistent with the existing GNU-stat and Linux-process assumptions. This patch changes permission and ownership checks authorizing script execution as well as process/session tracking (server/llmlaunch/commands.js:62, :112). Those execution-boundary changes meet this pass's privilege stop condition. Leaving this open for the maintainer's decision. (Review notes prepared with Claude Code on behalf of the maintainer.)

@ezoushen
ezoushen force-pushed the fix/macos-launchers branch from 432abbc to ec9d6f9 Compare October 23, 2026 02:16
@ezoushen

Copy link
Copy Markdown
Contributor Author

Thanks for the note on the execution-boundary checks. Their meaning is unchanged: a script or directory that is world-writable, not owned by the user, outside the home or a symlink is refused, and so is one whose mode or owner cannot be read. I pushed tests that pin the last part, each case isolating one check (mode unreadable, owner unreadable, both, another owner): Start and Stop refuse with exit 126 and the matching reason, and making an unreadable mode fail open, or dropping the owner check, makes the new test fail. Rebased on main (df6a223); the full server suite shows no failure that main does not also have on a Mac. Details are in the updated description.

@MiaAI-Lab

Copy link
Copy Markdown
Owner

The updated launcher suite passes all 24 tests on Linux, including new refusals for unreadable mode, unreadable owner and another owner (server/llmlaunch/__tests__/launcher.test.js:501). The GNU/BSD stat fallback addresses the documented macOS assumptions (server/llmlaunch/commands.js:54). This still changes the permission checks authorizing script execution and its session tracking (commands.js:112), meeting this pass's privilege stop condition. Leaving this open for maintainer approval. (Review notes prepared with Claude Code on behalf of the maintainer.)

The shell sparkDash runs over SSH for a model launcher read /proc, used GNU
stat -c and started the script with setsid. None exist on macOS, so every
Start and Stop ended at "refusing: <dir> is world-writable".

Read process facts from /proc where it exists and from ps elsewhere, fall back
to BSD stat -f, and use perl's POSIX::setsid when there is no setsid. ps start
times are read with LC_ALL=C. Linux units run the same commands as before.

launcher.test.js: 14 of 21 failed on macOS before; 23 of 23 pass on macOS and
on Linux, including two new tests (recorded start time, log size cap).
@ezoushen
ezoushen force-pushed the fix/macos-launchers branch from ec9d6f9 to ae677bb Compare October 23, 2026 15:54
@ezoushen

Copy link
Copy Markdown
Contributor Author

Understood that this needs a maintainer's approval. To make that review quick I added the exact delta for the privilege-relevant parts to "Merge risk" in the description: authorization (wwrite/check_script) only changes how mode and owner are read (GNU then BSD stat) with identical decisions and fail-closed behaviour pinned by the four refusal cases, and the session-tracking change (alive(): kill -0 instead of /proc/<pid>) decides only whether a second Start launches another copy, never whether a script may run. Rebased on 458cd06. The engine PRs (#265, #266) no longer include this commit, so they are not held behind it.

@MiaAI-Lab

Copy link
Copy Markdown
Owner

Re-checked the updated head ae677bb: typecheck passes and the full server run is 1115/1132 pass with one websocket-initial-snapshot failure that passes when run on its own (it was run next to two other suites, so a load flake rather than a launcher regression). Your delta summary matches the code: the mode and owner reads fall back from GNU to BSD stat and still refuse when neither can be read (server/llmlaunch/commands.js:54-58, :112), and alive() now uses kill -0 on the leader pid (:82). Because this changes the checks that authorise script execution and the session tracking that gates a second Start, it stays under the privilege stop condition and is left open for the maintainer's decision. (Review notes prepared with Claude Code on behalf of the maintainer.)

MiaAI-Lab added a commit that referenced this pull request Oct 24, 2026
Merged with --no-ff to keep the contributor's commits and authorship.
CHANGELOG and launcher test helper conflicts resolved by hand; credits added
for #218 and #264.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@MiaAI-Lab

Copy link
Copy Markdown
Owner

Now on main as 9a67b41 (cherry-picked with -x, authorship preserved; CHANGELOG credits @ezoushen). The portable /proc/ps, GNU/BSD stat and newsid helpers are in server/llmlaunch/commands.js:51-70, and the refusal paths still fail closed (:112-125). It was approved for merge by the maintainer. Launcher suite passes (27 tests) and the full typecheck, build and test run is green on main. Closing as completed. (Review notes prepared with Claude Code on behalf of the maintainer.)

@MiaAI-Lab MiaAI-Lab closed this Oct 24, 2026
MiaAI-Lab added a commit that referenced this pull request Oct 24, 2026
The drain change from #218 passes GNU-only --pid to tail, so on macOS the
follower exited at once and a Start printed nothing and never emitted the
exit marker, undoing the #264 fix. Probe for --pid and otherwise use the
earlier timed shutdown. Regression test uses a tail that rejects --pid.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants