From c7b17c455bb545fdfd54a00716331918fc88840d Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 20 Jul 2026 06:05:28 -0700 Subject: [PATCH] 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 - 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> --- unsloth_cli/commands/start.py | 116 ++++++++++--- unsloth_cli/tests/test_start.py | 277 ++++++++++++++++++++++++++++++-- 2 files changed, 356 insertions(+), 37 deletions(-) diff --git a/unsloth_cli/commands/start.py b/unsloth_cli/commands/start.py index 707c2b3c90..3d73df65be 100644 --- a/unsloth_cli/commands/start.py +++ b/unsloth_cli/commands/start.py @@ -192,7 +192,7 @@ _OPENCODE_NATIVE_AUTO_MIN_VERSION = (1, 17, 12) def _opencode_supports_native_auto() -> bool: - executable = shutil.which("opencode") + executable = _which_with_install_dirs("opencode") if executable is None: # No local binary: a --no-launch recipe may run elsewhere, and _run installs the # current release on launch -- either way assume native --auto is available. @@ -826,7 +826,7 @@ def _resolve_model( ) if requested and match is None: typer.echo( - f"Ensuring {requested} is loaded with the requested settings…" + f"Loading {requested} - please wait…" if load_has_overrides else f"Loading {requested} on the Unsloth server (this can take a while)…" ) @@ -902,17 +902,21 @@ def _require_gguf_for_codex(base: str, key: str, model_id: str) -> None: _DYNAMIC_SECTIONS_FLAG = "--exclude-dynamic-system-prompt-sections" -# Session overlay applied via `claude --settings`; suppresses the attribution header -# for THIS run only (no ~/.claude write) so llama.cpp KV-cache reuse is preserved. It -# reinforces the CLAUDE_CODE_ATTRIBUTION_HEADER env var on builds that read the setting -# only from settings.json. -_CLAUDE_SETTINGS_OVERLAY = '{"env":{"CLAUDE_CODE_ATTRIBUTION_HEADER":"0"}}' + + +def _claude_settings_overlay(model_id: str) -> str: + # Session-only `claude --settings` overlay (command-line tier, no ~/.claude write): + # suppress the attribution header, and pin availableModels to the served model so a + # user allowlist can't reject it. The pin must be non-empty; [] is ignored. + return json.dumps( + {"env": {"CLAUDE_CODE_ATTRIBUTION_HEADER": "0"}, "availableModels": [model_id]} + ) def _claude_version() -> Optional[tuple]: # None = no local `claude` (a --no-launch printout for another machine; assume a # current build). An unparseable version is treated as too old for the new flags. - executable = shutil.which("claude") + executable = _which_with_install_dirs("claude") if executable is None: return None try: @@ -929,16 +933,14 @@ def _claude_version() -> Optional[tuple]: return (0,) -def _claude_flags() -> list: - # Both knobs preserve llama.cpp KV-cache reuse: --exclude-dynamic-system-prompt-sections - # moves per-session context out of the system prompt, and --settings suppresses the - # attribution header for this session only (no persistent ~/.claude write; the env var - # sets it too). Claude Code < 2.1.98 aborts on unknown flags, so gate on the version; - # no local binary means a printout for another machine, so assume a current build. +def _claude_flags(model_id: str) -> list: + # KV-cache-preserving flags: move per-session context out of the system prompt and pass + # the session overlay. claude < 2.1.98 rejects unknown flags; no local binary means a + # printout for another machine, so assume a current build. version = _claude_version() if version is not None and version < (2, 1, 98): return [] - return [_DYNAMIC_SECTIONS_FLAG, "--settings", _CLAUDE_SETTINGS_OVERLAY] + return [_DYNAMIC_SECTIONS_FLAG, "--settings", _claude_settings_overlay(model_id)] def _merge_codex_config(existing: str, base: str) -> str: @@ -971,7 +973,7 @@ _CODEX_MODEL_CATALOG_MIN_VERSION = (0, 110, 0) def _codex_supports_model_catalog() -> bool: - executable = shutil.which("codex") + executable = _which_with_install_dirs("codex") if executable is None: # A --no-launch recipe may be copied to another machine; assume a current Codex. return True @@ -1202,6 +1204,53 @@ def _refresh_windows_path() -> None: os.environ["PATH"] = os.pathsep.join(entries) +def _augment_path_with_install_dirs() -> None: + # Append known install dirs to PATH so a freshly installed agent resolves without a new + # shell: some installers write the binary but not PATH (claude drops ~/.local/bin and + # only prints a note; npm -g shims land in %APPDATA%\npm). Appended, so precedence holds. + try: + home = Path.home() + except (RuntimeError, OSError): + return + candidates = [home / ".local" / "bin"] + if os.name == "nt": + appdata = os.environ.get("APPDATA") + if appdata: + candidates.append(Path(appdata) / "npm") + current = os.environ.get("PATH") + if current is None: + # PATH unset: shutil.which() and exec*p* fall back to os.defpath (e.g. /bin:/usr/bin), so + # keep that default instead of collapsing to just the install dirs (which would hide a + # system-installed agent and strip the launched child's normal PATH). An explicitly empty + # PATH is left as-is: like shutil.which, it means "search nothing", not os.defpath. + current = os.defpath + seen = {os.path.normcase(entry) for entry in current.split(os.pathsep) if entry} + additions = [ + str(directory) + for directory in candidates + if directory.is_dir() and os.path.normcase(str(directory)) not in seen + ] + if additions: + os.environ["PATH"] = os.pathsep.join([current, *additions] if current else additions) + + +def _which_with_install_dirs(name: str) -> Optional[str]: + # shutil.which(name), but searching the known agent install dirs too, so a version probe + # resolves the same binary _launch() will (it augments PATH before it runs). Without this an + # agent present only in ~/.local/bin / %APPDATA%\npm is missed, wrongly assumed current, and + # launched with flags an older build rejects. PATH is restored afterward: only _launch() + # should persist the augmentation for the child process. + original = os.environ.get("PATH") + _augment_path_with_install_dirs() + try: + return shutil.which(name) + finally: + if original is None: + os.environ.pop("PATH", None) + else: + os.environ["PATH"] = original + + def _install_source(install_hint: str) -> Optional[str]: """The first http(s) URL an install hint fetches, or None (e.g. an npm install).""" match = re.search(r"https?://[^\s'\")]+", install_hint) @@ -1254,17 +1303,35 @@ def _install_agent(name: str, install_hint: str) -> Optional[str]: typer.secho(warning, fg = "yellow", err = True) if not typer.confirm(f"Install `{name}` now with `{install_hint}`?", default = False): return None - # Run each hint through the shell it is written for: PowerShell (irm | iex, or npm) - # on Windows, /bin/sh (curl | bash, or npm) everywhere else. + # Run each hint through its shell: PowerShell on Windows, /bin/sh elsewhere. + # -ExecutionPolicy Bypass is process-scoped (nothing persistent) so npm's npm.ps1 and + # irm | iex run under the Windows default Restricted policy instead of failing with a + # PSSecurityException. if os.name == "nt": - install_command = ["powershell", "-NoProfile", "-Command", install_hint] + install_command = [ + "powershell", + "-NoProfile", + "-ExecutionPolicy", + "Bypass", + "-Command", + install_hint, + ] else: install_command = ["/bin/sh", "-c", install_hint] if subprocess.run(install_command).returncode != 0: - _fail(f"Install command failed. Run it yourself, then re-run: {install_hint}") - # The installer just wrote PATH to the registry (Windows); pull it into this - # process so the freshly installed agent resolves without a shell restart. + message = f"Install command failed. Run it yourself, then re-run: {install_hint}" + if os.name == "nt": + # A hand-run retry can still hit the policy; point at the one-time per-user fix. + message += ( + "\nIf it fails because running scripts is disabled (PSSecurityException), " + "allow local scripts for your user, then retry:\n" + " Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned" + ) + _fail(message) + # Resolve the freshly installed agent without a shell restart: pull registry PATH + # (Windows) plus well-known install dirs the installer may not have added to PATH. _refresh_windows_path() + _augment_path_with_install_dirs() executable = shutil.which(name) if executable is None: _fail( @@ -1290,6 +1357,9 @@ def _launch( install_hint: str, unset_env: tuple = (), ) -> NoReturn: + # Resolve well-known install dirs (e.g. ~/.local/bin) first, so an already-installed + # agent not yet on PATH is found instead of prompting a needless reinstall. + _augment_path_with_install_dirs() executable = shutil.which(command[0]) or _install_agent(command[0], install_hint) if executable is None: _fail(f"`{command[0]}` not found on PATH. Install it with: {install_hint}") @@ -1780,7 +1850,7 @@ def claude( "claude", "--model", model_id, - *_claude_flags(), + *_claude_flags(model_id), *_yolo_command_flags("claude", yolo), *ctx.args, ] diff --git a/unsloth_cli/tests/test_start.py b/unsloth_cli/tests/test_start.py index 227918f63e..1e03d390d1 100644 --- a/unsloth_cli/tests/test_start.py +++ b/unsloth_cli/tests/test_start.py @@ -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 ` (e.g. `unsloth start claude --resume `) 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.