Merge pull request #137 from Yif-Yang/fix/claude-tempdir-cleanup

fix(claude): use cleanup-tolerant temporary directory

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:

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)