Give agent probes the augmented PATH and cover an existing unwritable temp root
Two gaps in the previous two commits. _which_with_install_dirs restores PATH before returning, so a shim it resolved through Studio's managed Node could not find that node when the probe actually ran it. That broke `opencode debug config` on the --as-subagent path and made the claude and codex version probes report an unsupported build, which changes the flags the agent is launched with. Probes now build their environment with _probe_env, which augments PATH for the child without leaving it set in this process. The unwritable-root fallback only covered mkdir. When the temp root already exists but cannot be written, mkdir(exist_ok = True) succeeds and the failure moves to .cleanup.lock inside _short_ephemeral_session. The whole setup is now inside the guard, so either failure falls back to the system temp dir. unsloth_cli/tests/test_start.py: 388 passed.
This commit is contained in:
parent
d999f22f04
commit
cab05dac32
2 changed files with 68 additions and 17 deletions
|
|
@ -385,6 +385,7 @@ def _opencode_supports_native_auto() -> bool:
|
|||
text = True,
|
||||
timeout = 10,
|
||||
stderr = subprocess.DEVNULL,
|
||||
env = _probe_env(),
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
|
@ -1677,7 +1678,7 @@ def _claude_version() -> Optional[tuple]:
|
|||
return None
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[executable, "--version"], capture_output = True, text = True, timeout = 10
|
||||
[executable, "--version"], capture_output = True, text = True, timeout = 10, env = _probe_env()
|
||||
)
|
||||
# Pull the X.Y.Z out of the output rather than assuming it is the first token.
|
||||
# claude prints it first today ("2.1.98 (Claude Code)"), but a format change
|
||||
|
|
@ -1754,7 +1755,8 @@ def _codex_supports_model_catalog() -> bool:
|
|||
return True
|
||||
try:
|
||||
output = subprocess.check_output(
|
||||
[executable, "--version"], text = True, timeout = 10, stderr = subprocess.DEVNULL
|
||||
[executable, "--version"], text = True, timeout = 10, stderr = subprocess.DEVNULL,
|
||||
env = _probe_env(),
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
|
|
@ -2074,8 +2076,7 @@ def _opencode_subagent_inline_config(path: Path, permission: dict) -> dict:
|
|||
err = True,
|
||||
)
|
||||
else:
|
||||
env = os.environ.copy()
|
||||
env["OPENCODE_CONFIG"] = _agent_config_path(path, ["opencode"])
|
||||
env = _probe_env(OPENCODE_CONFIG = _agent_config_path(path, ["opencode"]))
|
||||
try:
|
||||
resolved = subprocess.run(
|
||||
[executable, "debug", "config"],
|
||||
|
|
@ -2485,6 +2486,23 @@ def _augment_path_with_install_dirs() -> None:
|
|||
os.environ["PATH"] = os.pathsep.join(parts)
|
||||
|
||||
|
||||
def _probe_env(**extra: str) -> dict:
|
||||
"""Environment for probes that RUN a resolved shim.
|
||||
|
||||
_which_with_install_dirs restores PATH before returning, so a shim backed by Studio's
|
||||
managed Node would not find that node when executed.
|
||||
"""
|
||||
original = os.environ.get("PATH")
|
||||
_augment_path_with_install_dirs()
|
||||
env = os.environ.copy()
|
||||
if original is None:
|
||||
os.environ.pop("PATH", None)
|
||||
else:
|
||||
os.environ["PATH"] = original
|
||||
env.update(extra)
|
||||
return env
|
||||
|
||||
|
||||
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
|
||||
|
|
@ -2803,20 +2821,17 @@ def _temporary_agent_config(prefix: str):
|
|||
# locked session helper: a wrapper killed before its finally runs leaves a home that
|
||||
# the next launch reclaims, and the lock keeps a live session from being swept.
|
||||
temp_root = _agents_config_root() / ".tmp"
|
||||
try:
|
||||
temp_root.mkdir(parents = True, exist_ok = True, mode = 0o700)
|
||||
except OSError:
|
||||
# Attaching to a remote or already-running Studio does not need a local auth tree,
|
||||
# so it may be absent or read-only; the key cache degrades the same way. Fall back
|
||||
# to the system temp dir, which is where these homes lived before. No reclamation
|
||||
# there, but the OS prunes it.
|
||||
path = Path(tempfile.mkdtemp(prefix = prefix))
|
||||
with contextlib.ExitStack() as stack:
|
||||
try:
|
||||
yield path
|
||||
finally:
|
||||
shutil.rmtree(path, ignore_errors = True)
|
||||
return
|
||||
with _short_ephemeral_session(temp_root, prefix) as path:
|
||||
temp_root.mkdir(parents = True, exist_ok = True, mode = 0o700)
|
||||
path = stack.enter_context(_short_ephemeral_session(temp_root, prefix))
|
||||
except OSError:
|
||||
# Attaching to a remote or already-running Studio does not need a local auth
|
||||
# tree, so it may be absent, read-only, or owned by someone else; the key cache
|
||||
# degrades the same way. Fall back to the system temp dir, where these homes
|
||||
# lived before. No reclamation there, but the OS prunes it.
|
||||
path = Path(tempfile.mkdtemp(prefix = prefix))
|
||||
stack.callback(shutil.rmtree, path, ignore_errors = True)
|
||||
yield path
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -5219,6 +5219,42 @@ def test_non_codex_agents_keep_the_studio_private_root(monkeypatch, tmp_path):
|
|||
assert start._ephemeral_session_prefix("claude", None) == "unsloth-claude-"
|
||||
|
||||
|
||||
def test_session_config_falls_back_when_existing_temp_root_is_unwritable(monkeypatch, tmp_path):
|
||||
# mkdir(exist_ok = True) succeeds on a root that already exists but cannot be written,
|
||||
# so the lock file is the first thing to fail.
|
||||
agents = tmp_path / "agents"
|
||||
temp_root = agents / ".tmp"
|
||||
temp_root.mkdir(parents = True)
|
||||
os.chmod(temp_root, 0o500)
|
||||
monkeypatch.setattr(start, "_agents_config_root", lambda: agents)
|
||||
|
||||
try:
|
||||
with start._session_config("claude", launch = True) as home:
|
||||
assert home.exists()
|
||||
assert temp_root not in home.parents
|
||||
finally:
|
||||
os.chmod(temp_root, 0o700)
|
||||
assert not home.exists()
|
||||
|
||||
|
||||
def test_probe_env_carries_install_dirs_and_restores_path(monkeypatch, tmp_path):
|
||||
# A shim resolved via Studio's managed Node needs that node on PATH when it runs.
|
||||
managed_bin = tmp_path / "node" / "bin"
|
||||
managed_bin.mkdir(parents = True)
|
||||
monkeypatch.setattr(
|
||||
start,
|
||||
"_managed_node_tools",
|
||||
lambda: (managed_bin / "node", managed_bin / "npm", True),
|
||||
)
|
||||
before = os.environ.get("PATH")
|
||||
|
||||
env = start._probe_env(OPENCODE_CONFIG = "/tmp/cfg.json")
|
||||
|
||||
assert str(managed_bin) in env["PATH"]
|
||||
assert env["OPENCODE_CONFIG"] == "/tmp/cfg.json"
|
||||
assert os.environ.get("PATH") == before
|
||||
|
||||
|
||||
def test_session_config_falls_back_when_studio_auth_root_is_unwritable(monkeypatch, tmp_path):
|
||||
# Attaching to a remote or already-running Studio does not need a local auth tree, so
|
||||
# an absent or read-only one must not stop the launch.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue