Fix unsloth start on Windows: agent install, PATH resolution, and local model selection (#7257)

* unsloth start: fix Windows agent install/launch and local model selection

- claude: pin availableModels to the served model in the session --settings
  overlay so a user's ~/.claude/settings.json allowlist no longer substitutes
  the org default for the local Unsloth model. The allowlist covers --model,
  ANTHROPIC_MODEL and the model setting, and an empty [] is ignored, so the pin
  lists the model explicitly.
- installs: run the Windows installer under -ExecutionPolicy Bypass
  (process-scoped, nothing persistent) so npm's npm.ps1 and irm|iex scripts run
  under the default Restricted policy; on failure, hint at Set-ExecutionPolicy
  -Scope CurrentUser -ExecutionPolicy RemoteSigned for a hand-run retry.
- PATH: resolve agents installed to ~/.local/bin (claude) and %APPDATA%\npm (npm
  agents) in-process, so a fresh install launches without opening a new shell and
  an already-installed agent is not re-prompted for install.
- load message: "Loading <model> - please wait" while a model loads.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* unsloth start: resolve agent version against the launch PATH

The claude/codex/opencode version probes ran shutil.which while building the
command, before _launch augments PATH with the known install dirs. An agent
present only in ~/.local/bin or %APPDATA%\npm was therefore missed, assumed to
be a current build, and launched with flags an older build rejects (claude
aborts on the unknown flags). Route the three probes through a new
_which_with_install_dirs() so each resolves the same binary _launch will,
restoring PATH afterward so only _launch persists the augmentation.

Add regression tests for the three probes (POSIX and the Windows npm dir) and
make the Windows-branch tests run on POSIX hosts (pinning Path to the native
flavour so a simulated os.name does not make pathlib build WindowsPath).

* unsloth start: keep os.defpath when augmenting an unset PATH

_augment_path_with_install_dirs collapsed an unset PATH to just the install
dirs, dropping the os.defpath fallback (/bin:/usr/bin) that shutil.which and
exec*p* use when PATH is absent. A system-installed agent then looked missing
and the launched child lost its normal PATH. Seed os.defpath when PATH is
unset; an explicitly empty PATH is left as-is (search nothing), matching
shutil.which. Add regression tests for the augment helper and the version-probe
wrapper.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Daniel Han 2026-07-20 06:05:28 -07:00 committed by GitHub
commit c7b17c455b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 356 additions and 37 deletions

View file

@ -61,36 +61,73 @@ def _fake_claude(monkeypatch, version_output: str) -> None:
)
def _path_aware_which(binaries: dict):
# A shutil.which fake that resolves a name only when its directory is on PATH at call time.
# Lets a test prove a version probe augments PATH before resolving: an agent present only in
# an install dir (~/.local/bin, %APPDATA%\npm) must still be found and version-checked.
def _which(name):
directory = binaries.get(name)
if directory is None:
return None
entries = os.environ.get("PATH", "").split(os.pathsep)
# os.path.join (not Path()) so this works when a test has flipped os.name to "nt": under
# a simulated os.name, pathlib would build the non-native flavour and raise.
return os.path.join(str(directory), name) if str(directory) in entries else None
return _which
def _simulate_windows(monkeypatch) -> None:
# Exercise the `os.name == "nt"` branch on any host. Flipping os.name alone makes pathlib
# pick the non-native flavour (WindowsPath on POSIX, PosixPath on Windows) when a Path is
# constructed, which raises; pin Path to the host-native class (captured before the flip)
# so the branch logic runs without that crash. Keeps these tests green on Linux/Mac/WSL too.
monkeypatch.setattr(start, "Path", type(Path()))
monkeypatch.setattr(start.os, "name", "nt")
def test_claude_flags_passed_to_supported_claude(monkeypatch):
_fake_claude(monkeypatch, "2.1.98 (Claude Code)\n")
assert start._claude_flags() == [
assert start._claude_flags(MODEL["id"]) == [
"--exclude-dynamic-system-prompt-sections",
"--settings",
start._CLAUDE_SETTINGS_OVERLAY,
start._claude_settings_overlay(MODEL["id"]),
]
def test_claude_flags_skipped_on_old_claude(monkeypatch):
_fake_claude(monkeypatch, "2.0.14 (Claude Code)\n")
assert start._claude_flags() == []
assert start._claude_flags(MODEL["id"]) == []
def test_claude_flags_skipped_on_unparseable_version(monkeypatch):
_fake_claude(monkeypatch, "weird build string\n")
assert start._claude_flags() == []
assert start._claude_flags(MODEL["id"]) == []
def test_claude_flags_detected_when_version_not_first_token(monkeypatch):
# The X.Y.Z is pulled from anywhere in the output, so a format change (version not
# the first token) doesn't silently drop the optimization flags.
_fake_claude(monkeypatch, "claude version 2.1.98\n")
assert start._claude_flags() == [
assert start._claude_flags(MODEL["id"]) == [
"--exclude-dynamic-system-prompt-sections",
"--settings",
start._CLAUDE_SETTINGS_OVERLAY,
start._claude_settings_overlay(MODEL["id"]),
]
def test_claude_settings_overlay_pins_served_model():
# The session overlay must pin availableModels to the served model: a user's allowlist
# in ~/.claude/settings.json otherwise rejects the Unsloth --model ("restricted by your
# organization's settings"), and no env var can bypass it. The override must be a
# NON-EMPTY array to take effect (an empty [] is ignored and the user's list still
# applies), so it lists exactly this model, for this session only.
overlay = json.loads(start._claude_settings_overlay(MODEL["id"]))
assert overlay["availableModels"] == [MODEL["id"]]
# The attribution-header suppression is preserved alongside it.
assert overlay["env"]["CLAUDE_CODE_ATTRIBUTION_HEADER"] == "0"
def test_install_agent_prompts_then_installs(monkeypatch):
# TTY + yes: run the documented install command, then re-resolve the now-present binary.
monkeypatch.setattr(start.os, "name", "posix")
@ -126,7 +163,52 @@ def test_install_agent_uses_powershell_on_windows(monkeypatch):
executable = start._install_agent("hermes", install_hint)
assert executable == r"C:\Users\samle\bin\hermes.exe"
assert ran == [["powershell", "-NoProfile", "-Command", install_hint]]
# -ExecutionPolicy Bypass (process-scoped) lets npm's npm.ps1 wrapper and irm|iex
# scripts run even when the machine policy is the Windows default Restricted.
assert ran == [
["powershell", "-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", install_hint]
]
def test_install_agent_windows_failure_hints_execution_policy(monkeypatch, capsys):
# A failed install on Windows points the user at the per-user execution-policy fix:
# our subprocess bypasses the policy, but their own shell may still block npm.ps1
# (PSSecurityException) when they run the install by hand.
monkeypatch.setattr(start.os, "name", "nt")
monkeypatch.setattr(start.sys, "stdin", SimpleNamespace(isatty = lambda: True))
monkeypatch.setattr(start.typer, "confirm", lambda *a, **k: True)
monkeypatch.setattr(
start.subprocess,
"run",
lambda *a, **k: SimpleNamespace(returncode = 1),
)
monkeypatch.setattr(start.shutil, "which", lambda _: None)
with pytest.raises(start.typer.Exit):
start._install_agent("codex", "npm install -g @openai/codex")
err = capsys.readouterr().err
assert "Install command failed" in err
assert "Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned" in err
def test_install_agent_posix_failure_omits_execution_policy_hint(monkeypatch, capsys):
# The execution-policy hint is Windows-only; a POSIX install failure must not mention it.
monkeypatch.setattr(start.os, "name", "posix")
monkeypatch.setattr(start.sys, "stdin", SimpleNamespace(isatty = lambda: True))
monkeypatch.setattr(start.typer, "confirm", lambda *a, **k: True)
monkeypatch.setattr(
start.subprocess,
"run",
lambda *a, **k: SimpleNamespace(returncode = 1),
)
with pytest.raises(start.typer.Exit):
start._install_agent("codex", "npm install -g @openai/codex")
err = capsys.readouterr().err
assert "Install command failed" in err
assert "Set-ExecutionPolicy" not in err
def test_install_agent_warns_remote_installer_is_unverified_third_party(monkeypatch, capsys):
@ -254,6 +336,172 @@ def test_refresh_windows_path_merges_registry_hives(monkeypatch):
]
def test_augment_path_adds_existing_local_bin(monkeypatch, tmp_path):
# Claude's installer drops its binary in ~/.local/bin but only *suggests* adding it to
# PATH, so Unsloth appends it in-process to resolve the freshly installed agent.
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata")) # skip the npm candidate
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
start._augment_path_with_install_dirs()
entries = os.environ["PATH"].split(os.pathsep)
assert str(local_bin) in entries
# Appended (lowest precedence), so it never shadows an existing PATH entry.
assert entries[-1] == str(local_bin)
def test_augment_path_skips_missing_and_duplicate_dirs(monkeypatch, tmp_path):
# A non-existent ~/.local/bin is not added; an already-present one is not duplicated.
monkeypatch.setattr(start.Path, "home", lambda: tmp_path) # no .local/bin created yet
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata")) # skip the npm candidate
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
start._augment_path_with_install_dirs()
assert os.environ["PATH"] == str(tmp_path / "existing")
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setenv("PATH", os.pathsep.join([str(tmp_path / "existing"), str(local_bin)]))
start._augment_path_with_install_dirs()
assert os.environ["PATH"].split(os.pathsep).count(str(local_bin)) == 1
def test_augment_path_adds_npm_global_bin_on_windows(monkeypatch, tmp_path):
# npm -g shims (codex/opencode/pi) land in %APPDATA%\npm on Windows; add it so a freshly
# installed npm agent resolves even when that dir isn't on PATH yet.
_simulate_windows(monkeypatch)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path) # no ~/.local/bin created
npm_dir = tmp_path / "Roaming" / "npm"
npm_dir.mkdir(parents = True)
monkeypatch.setenv("APPDATA", str(tmp_path / "Roaming"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
start._augment_path_with_install_dirs()
assert str(npm_dir) in os.environ["PATH"].split(os.pathsep)
def test_which_with_install_dirs_finds_agent_and_restores_path(monkeypatch, tmp_path):
# The probe helper resolves against the augmented PATH but must NOT persist it: only
# _launch() should mutate PATH for the child process. Here `claude` is only in ~/.local/bin.
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata")) # skip the npm candidate
original = str(tmp_path / "existing")
monkeypatch.setenv("PATH", original) # local_bin NOT on PATH yet
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"claude": local_bin}))
assert start._which_with_install_dirs("claude") == str(local_bin / "claude")
assert os.environ["PATH"] == original # restored, no global pollution
def test_claude_flags_probes_old_agent_only_in_install_dir(monkeypatch, tmp_path):
# Regression: the version probe must augment PATH before resolving, so an OLD claude present
# only in ~/.local/bin (not yet on PATH) is detected as old and the unsupported flags are
# dropped -- the same binary _launch() will run. Before the fix the probe saw no binary,
# assumed a current build, and emitted flags the old claude rejects.
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"claude": local_bin}))
monkeypatch.setattr(
start.subprocess, "run", lambda *a, **k: SimpleNamespace(stdout = "2.0.14 (Claude Code)\n")
)
assert start._claude_flags(MODEL["id"]) == []
def test_claude_flags_detects_supported_agent_only_in_install_dir(monkeypatch, tmp_path):
# The counterpart: a SUPPORTED claude present only in ~/.local/bin is now resolved and gets
# the flags, instead of being missed and (coincidentally) also assumed current.
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"claude": local_bin}))
monkeypatch.setattr(
start.subprocess, "run", lambda *a, **k: SimpleNamespace(stdout = "2.1.98 (Claude Code)\n")
)
assert start._claude_flags(MODEL["id"]) == [
"--exclude-dynamic-system-prompt-sections",
"--settings",
start._claude_settings_overlay(MODEL["id"]),
]
def test_claude_flags_probes_npm_install_dir_on_windows(monkeypatch, tmp_path):
# npm -g shims land in %APPDATA%\npm on Windows; an old claude there (not on PATH) must still
# be version-checked so the unsupported flags are dropped.
_simulate_windows(monkeypatch)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path) # no ~/.local/bin created
npm_dir = tmp_path / "Roaming" / "npm"
npm_dir.mkdir(parents = True)
monkeypatch.setenv("APPDATA", str(tmp_path / "Roaming"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"claude": npm_dir}))
monkeypatch.setattr(
start.subprocess, "run", lambda *a, **k: SimpleNamespace(stdout = "2.0.14 (Claude Code)\n")
)
assert start._claude_flags(MODEL["id"]) == []
def test_codex_catalog_probes_old_codex_only_in_install_dir(monkeypatch, tmp_path):
# Same ordering fix for codex: an old codex present only in an install dir is detected so the
# model-catalog config is omitted (the old binary can't consume it).
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"codex": local_bin}))
monkeypatch.setattr(start.subprocess, "check_output", lambda *a, **k: "codex-cli 0.109.0")
assert start._codex_supports_model_catalog() is False
def test_opencode_native_auto_probes_old_opencode_only_in_install_dir(monkeypatch, tmp_path):
# Same ordering fix for opencode: an old opencode present only in an install dir is detected
# so native --auto is not assumed (the old binary rejects it).
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.setenv("PATH", str(tmp_path / "existing"))
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"opencode": local_bin}))
monkeypatch.setattr(start.subprocess, "check_output", lambda *a, **k: "1.17.11")
assert start._opencode_supports_native_auto() is False
def test_augment_path_preserves_defpath_when_path_unset(monkeypatch, tmp_path):
# PATH unset: shutil.which() and exec*p* fall back to os.defpath (e.g. /bin:/usr/bin), so the
# augmentation must keep those default dirs instead of collapsing to just the install dir
# (which would hide a system-installed agent and strip the launched child's normal PATH).
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.delenv("PATH", raising = False)
start._augment_path_with_install_dirs()
entries = os.environ["PATH"].split(os.pathsep)
for default_dir in os.defpath.split(os.pathsep):
if default_dir:
assert default_dir in entries
assert str(local_bin) in entries
def test_which_with_install_dirs_keeps_defpath_when_path_unset(monkeypatch, tmp_path):
# With PATH unset, a system agent on os.defpath (e.g. /usr/bin) must still resolve; the
# install-dir augmentation must not drop the default search path. PATH is restored to unset.
local_bin = tmp_path / ".local" / "bin"
local_bin.mkdir(parents = True)
monkeypatch.setattr(start.Path, "home", lambda: tmp_path)
monkeypatch.setenv("APPDATA", str(tmp_path / "no-appdata"))
monkeypatch.delenv("PATH", raising = False)
sysdir = next(part for part in reversed(os.defpath.split(os.pathsep)) if part)
monkeypatch.setattr(start.shutil, "which", _path_aware_which({"claude": Path(sysdir)}))
assert start._which_with_install_dirs("claude") == os.path.join(sysdir, "claude")
assert "PATH" not in os.environ
def test_install_agent_declined_returns_none(monkeypatch):
# TTY + no: never runs anything; caller falls back to the print-hint failure.
monkeypatch.setattr(start.sys, "stdin", SimpleNamespace(isatty = lambda: True))
@ -451,7 +699,7 @@ def test_connect_claude_launch_scrubs_conflicting_auth_env(fake_studio, monkeypa
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-anthropic-stale")
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "oauth-stale")
monkeypatch.setattr(start.shutil, "which", lambda _: "/usr/local/bin/claude")
monkeypatch.setattr(start, "_claude_flags", lambda: [])
monkeypatch.setattr(start, "_claude_flags", lambda *a, **k: [])
def run(command, env):
captured["command"] = command
@ -486,7 +734,7 @@ def test_connect_claude_windows_shim_from_wsl_bridges_env(fake_studio, monkeypat
monkeypatch.setattr(
start.shutil, "which", lambda _: "/mnt/c/Users/samle/AppData/Roaming/npm/claude"
)
monkeypatch.setattr(start, "_claude_flags", lambda: [])
monkeypatch.setattr(start, "_claude_flags", lambda *a, **k: [])
def run(command, env):
captured["command"] = command
@ -2175,8 +2423,9 @@ def test_powershell_quote_single_quotes_json():
# keeps the embedded double quotes literal (list2cmdline's backslashes would not).
assert start._powershell_quote("--settings") == "--settings"
assert start._powershell_quote("unsloth/gemma-4-26B") == "unsloth/gemma-4-26B"
quoted = start._powershell_quote(start._CLAUDE_SETTINGS_OVERLAY)
assert quoted == "'" + start._CLAUDE_SETTINGS_OVERLAY + "'"
overlay = start._claude_settings_overlay("unsloth/gemma-4-26B")
quoted = start._powershell_quote(overlay)
assert quoted == "'" + overlay + "'"
assert "\\" not in quoted # no cmd.exe backslash escaping
assert start._powershell_quote("a'b") == "'a''b'" # embedded quote doubled
@ -2811,7 +3060,7 @@ def test_claude_launch_does_not_clear(fake_studio, monkeypatch):
calls = []
monkeypatch.setattr(start.click, "clear", lambda: calls.append("clear"))
monkeypatch.setattr(start.shutil, "which", lambda _: "/usr/local/bin/claude")
monkeypatch.setattr(start, "_claude_flags", lambda: [])
monkeypatch.setattr(start, "_claude_flags", lambda *a, **k: [])
monkeypatch.setattr(start.subprocess, "run", lambda command, env: SimpleNamespace(returncode = 0))
result = CliRunner().invoke(start.start_app, ["claude"])
assert result.exit_code == 0, result.output
@ -2988,7 +3237,7 @@ def test_persist_bare_opencode_launch_has_no_resume_token(fake_studio, monkeypat
def test_persist_bare_claude_launch_has_no_resume_token(fake_studio, monkeypatch):
monkeypatch.setattr(start.shutil, "which", lambda _: "/usr/local/bin/claude")
monkeypatch.setattr(start, "_claude_flags", lambda: [])
monkeypatch.setattr(start, "_claude_flags", lambda *a, **k: [])
captured = _capture_launch(monkeypatch, ["claude", "--persist"])
assert "--continue" not in captured["command"]
assert captured["command"][1:] == ["--model", MODEL["id"]]
@ -3152,7 +3401,7 @@ def test_native_resume_flag_passes_through_unchanged(fake_studio, monkeypatch):
# `--resume <id>` (e.g. `unsloth start claude --resume <guid>`) still flows
# through to the agent verbatim and is not swallowed as an Unsloth option.
monkeypatch.setattr(start.shutil, "which", lambda _: "/usr/local/bin/claude")
monkeypatch.setattr(start, "_claude_flags", lambda: [])
monkeypatch.setattr(start, "_claude_flags", lambda *a, **k: [])
captured = _capture_launch(monkeypatch, ["claude", "--resume", "some-session-guid"])
assert captured["command"][-2:] == ["--resume", "some-session-guid"]
# Unsloth never auto-appends its own resume token when the user drives resume.