From cb13d2201217aea8184a7b6554eb06c746c97b88 Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:35:15 +0900 Subject: [PATCH] fix(resolve-input): remove the temp directory when input resolution fails resolve_input cleaned up the InputHandler temp directory only for transitive ingest truncation. Any other failure (a zip that fails extraction, a failed clone, a download error after the temp directory exists) was re-raised with the directory still on disk. The graph then fails before temp_dir_for_cleanup reaches the CLI or MCP server, so no caller can remove it: a downloaded zip that fails extraction stays in the temp directory as download.zip, and a failed local zip leaves an empty skillspector_* directory, on every attempt. Call handler.cleanup() before re-raising any resolution error. Co-Authored-By: Claude Opus 5 Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com> --- src/skillspector/nodes/resolve_input.py | 5 ++- tests/nodes/test_resolve_input.py | 45 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/skillspector/nodes/resolve_input.py b/src/skillspector/nodes/resolve_input.py index 5ff22ac43..4b1b6616e 100644 --- a/src/skillspector/nodes/resolve_input.py +++ b/src/skillspector/nodes/resolve_input.py @@ -79,7 +79,10 @@ def resolve_input(state: SkillspectorState) -> dict[str, object]: exc.truncation.code, ) raise - except (ValueError, FileNotFoundError): + except Exception: + # The graph fails before returning temp_dir_for_cleanup, so no caller + # can remove a partial download or extraction directory afterwards. + handler.cleanup() raise if skill_path and isinstance(skill_path, str) and skill_path.strip(): diff --git a/tests/nodes/test_resolve_input.py b/tests/nodes/test_resolve_input.py index 7adc6b360..31c594ebb 100644 --- a/tests/nodes/test_resolve_input.py +++ b/tests/nodes/test_resolve_input.py @@ -15,8 +15,12 @@ """Tests for resolve_input node.""" +import io +import tempfile +import zipfile from pathlib import Path +import httpx import pytest from skillspector.input_handler import TransitiveIngestTruncatedError @@ -131,3 +135,44 @@ def cleanup(self) -> None: } assert cleaned == [True] assert "private/source" not in str(raised.value) + + +@pytest.mark.parametrize("source", ["local-zip", "downloaded-zip"]) +def test_failed_materialization_removes_the_handler_temp_dir( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, source: str +) -> None: + """An input that fails to materialize leaves no temp directory or download behind.""" + archive = io.BytesIO() + with zipfile.ZipFile(archive, "w") as bundle: + bundle.writestr("SKILL.md", "# Skill\n") + bundle.writestr("../outside.md", "escape\n") + created: list[Path] = [] + real_mkdtemp = tempfile.mkdtemp + + def mkdtemp(*args: str, **kwargs: str) -> str: + path = real_mkdtemp(*args, dir=tmp_path, **kwargs) + created.append(Path(path)) + return path + + monkeypatch.setattr("skillspector.input_handler.tempfile.mkdtemp", mkdtemp) + if source == "local-zip": + local_zip = tmp_path / "slip.zip" + local_zip.write_bytes(archive.getvalue()) + input_path = str(local_zip) + else: + real_client = httpx.Client + transport = httpx.MockTransport( + lambda _request: httpx.Response(200, content=archive.getvalue()) + ) + monkeypatch.setattr( + "skillspector.input_handler.httpx.Client", + lambda *args, **kwargs: real_client(*args, transport=transport, **kwargs), + ) + monkeypatch.setattr("skillspector.input_handler._is_private_ip", lambda _host: False) + input_path = "https://raw.githubusercontent.com/org/repo/main/skill.zip" + + with pytest.raises(ValueError, match="zip-slip"): + resolve_input({"input_path": input_path}) + + assert created + assert [path for path in created if path.exists()] == []