diff --git a/skillopt/model/claude_backend.py b/skillopt/model/claude_backend.py index 75020b1..373811a 100644 --- a/skillopt/model/claude_backend.py +++ b/skillopt/model/claude_backend.py @@ -243,11 +243,12 @@ def _assistant_message_schema_wrapper() -> str: def _run_claude_print(*, system: str, prompt: str, model: str, tools: list[dict[str, Any]] | None, tool_choice: str | dict[str, Any] | None, return_message: bool, timeout: int | None, attachments: list[dict[str, Any]] | None = None) -> tuple[str, dict[str, Any], dict[str, int]]: effort = _normalize_reasoning_effort(REASONING_EFFORT) - # Use mkdtemp + manual rmtree to avoid WinError 32 on Windows - # where the spawned claude/node subprocess still holds a handle - # to the temp directory when the context manager tries to clean up. - temp_dir = tempfile.mkdtemp(prefix="skillopt_claude_") - try: + # A lingering claude/node handle can make Windows cleanup raise WinError 32. + # Python 3.10+ supports suppressing cleanup failures while retaining + # TemporaryDirectory's normal lifecycle and best-effort removal behavior. + with tempfile.TemporaryDirectory( + prefix="skillopt_claude_", ignore_cleanup_errors=True, + ) as temp_dir: copied_attachments = _copy_attachments_to_temp(attachments or [], temp_dir) prompt_for_cli = _append_attachment_instructions(prompt, copied_attachments) cmd = [CLAUDE_BIN, "-p", "--output-format", "json", "--permission-mode", CLAUDE_PERMISSION_MODE, "--add-dir", temp_dir] @@ -291,8 +292,6 @@ def _run_claude_print(*, system: str, prompt: str, model: str, tools: list[dict[ raw_text, result_event = _extract_result(stream) usage_info = _usage_from_result(result_event) return raw_text, result_event or {}, usage_info - finally: - shutil.rmtree(temp_dir, ignore_errors=True) def _compat_message_from_payload(payload: Any) -> CompatAssistantMessage: diff --git a/tests/test_claude_backend_tempdir.py b/tests/test_claude_backend_tempdir.py new file mode 100644 index 0000000..ed0bcbd --- /dev/null +++ b/tests/test_claude_backend_tempdir.py @@ -0,0 +1,59 @@ +"""Regression coverage for Claude CLI temporary-directory cleanup.""" +from __future__ import annotations + +import json +import os +import shutil +import tempfile +from types import SimpleNamespace + +from skillopt.model import claude_backend + + +def test_claude_print_uses_cleanup_tolerant_temporary_directory(monkeypatch) -> None: + cleanup_modes = [] + subprocess_cwd = "" + + def simulated_windows_rmtree(cls, name, ignore_errors=False, repeated=False): + del cls, repeated + cleanup_modes.append(ignore_errors) + if not ignore_errors: + raise PermissionError(32, "directory is still in use", name) + shutil.rmtree(name, ignore_errors=True) + + def fake_run(*args, **kwargs): + nonlocal subprocess_cwd + subprocess_cwd = kwargs["cwd"] + payload = { + "type": "result", + "result": "ok", + "usage": {"input_tokens": 2, "output_tokens": 3}, + } + return SimpleNamespace( + returncode=0, + stdout=json.dumps(payload) + "\n", + stderr="", + ) + + monkeypatch.setattr( + tempfile.TemporaryDirectory, + "_rmtree", + classmethod(simulated_windows_rmtree), + ) + monkeypatch.setattr(claude_backend.subprocess, "run", fake_run) + + text, _, usage = claude_backend._run_claude_print( + system="system", + prompt="prompt", + model="", + tools=None, + tool_choice=None, + return_message=False, + timeout=10, + ) + + assert text == "ok" + assert usage == {"prompt_tokens": 2, "completion_tokens": 3, "total_tokens": 5} + assert cleanup_modes == [True] + assert subprocess_cwd + assert not os.path.exists(subprocess_cwd)