fix(resolve-input): remove the temp directory when input resolution fails

[kevin9327]

Problem

When an input fails to resolve after InputHandler has created its temp directory, the directory is never removed. This happens with a zip that fails extraction, a failed clone, or a download that errors. Each failed attempt leaves another skillspector_* directory in the system temp dir.

A local zip containing a ../ member, scanned through the CLI (Windows 11, Python 3.12.10, main at 4148ab3). The script lists skillspector_* in tempfile.gettempdir() before and after:

$ skillspector scan slip.zip --no-llm --format json
exit: 2
stderr: Error: Zip entry would escape extraction directory (zip-slip)
leaked temp dirs: ['skillspector_wbhv7hv1']

The same archive served from an allowed download host leaves the full download behind. This runs resolve_input with an httpx.MockTransport serving a 200 KB zip at https://raw.githubusercontent.com/org/repo/main/skill.zip:

== main
raised: Zip entry would escape extraction directory (zip-slip)
leaked: ['skillspector_gab_x39q']
    download.zip 200231
== this branch
raised: Zip entry would escape extraction directory (zip-slip)
leaked: []

Downloads are bounded by INGEST_MAX_BYTES (100 MiB), and a long-running skillspector mcp server accumulates one such directory per failed scan.

Cause

resolve_input calls handler.cleanup() only in the TransitiveIngestTruncatedError branch. Every other error is re-raised as-is:

except (ValueError, FileNotFoundError):
    raise

Once the node raises, the graph never returns temp_dir_for_cleanup, so cleanup_result in the CLI and MCP server has nothing to remove. _extract_zip and _clone_git delete their own subdirectory on failure, but the handler's temp directory (and a downloaded download.zip in it) belongs to the handler.

Fix

Call handler.cleanup() before re-raising any resolution error. The exception is unchanged. The truncation branch keeps its existing cleanup and logging. InputHandler.cleanup() is already best-effort and does nothing when no temp directory was created.

Tests

tests/nodes/test_resolve_input.py::test_failed_materialization_removes_the_handler_temp_dir uses the real InputHandler for a local zip and for a downloaded zip (served by httpx.MockTransport). tempfile.mkdtemp is redirected into tmp_path so the test sees exactly the directories the handler created. The test asserts the zip-slip error still propagates and none of those directories remain.

Against unmodified resolve_input.py:

E       AssertionError: assert [WindowsPath(...or_v0f_1p1r')] == []
E         Left contains one more item: WindowsPath('.../test_failed_materialization_re0/skillspector_v0f_1p1r')
E       AssertionError: assert [WindowsPath(...or_y8n28ymu')] == []
E         Left contains one more item: WindowsPath('.../test_failed_materialization_re1/skillspector_y8n28ymu')
FAILED tests/nodes/test_resolve_input.py::test_failed_materialization_removes_the_handler_temp_dir[local-zip]
FAILED tests/nodes/test_resolve_input.py::test_failed_materialization_removes_the_handler_temp_dir[downloaded-zip]
2 failed

With the fix, the whole file passes: 8 passed, 1 skipped (the skip is the existing symlink case on Windows). Branch coverage shows the new except path executed. The CLI reproduction above now reports leaked temp dirs: [].

Suite

pytest -m "not integration and not provider" -p no:randomly, same machine:

	main	this branch
tests/unit, tests/opencode, tests/test_*.py	10 failed, 1721 passed	10 failed, 1721 passed
tests/nodes	3 failed, 3588 passed, 4 errors	3 failed, 3590 passed, 4 errors

The +2 are the new cases. The failures and errors are identical on both sides and Windows-only (release and compare_scan_accuracy harnesses, CRLF fixtures, a backslash inside a POSIX file name, and parametrized test IDs over the Windows environment-variable length limit).

ruff check src/ tests/: All checks passed. ruff format --check src/ tests/: 246 files already formatted.

This is separate from #567, which covers read-only files that survive cleanup on Windows after a successful Git scan. The two touch different files.

🤖 Generated with Claude Code

[rng1995]

[SkillSpector Review]

Reviewed current head cb13d2201217aea8184a7b6554eb06c746c97b88, including the complete diff, surrounding resolver and InputHandler lifecycle, download/archive/clone failure paths, tests, and hosted checks.

Calling handler.cleanup() before re-raising ordinary resolution failures correctly closes the existing leak for the covered ZIP and download cases, and it preserves the original exception. One lifecycle gap still requires correction: the new except Exception path does not run for interruption or cancellation derived from BaseException. A Ctrl-C after the handler allocates its temporary directory can therefore leave _download.partial or download.zip; interruption during Git materialization can additionally leave a live child process and its tree. The inline finding describes the expected cleanup-and-reraise behavior and regression coverage.

The separate Windows read-only deletion limitation belongs to #567 and is not duplicated here. All completed hosted checks pass; test-unit is still running at review time.

[rng1995]

[P2] Clean up interrupted materializations too

KeyboardInterrupt and cancellation signals derived from BaseException bypass this handler. If interruption arrives after _get_temp_dir() during _download_file, the partial download and handler directory remain; during _clone_git, the child process and its tree can remain as well. Nearby extraction and single-file-copy cleanup already uses except BaseException specifically to release resources before re-raising. Please make failure cleanup cover interruption paths, ensure an interrupted clone terminates its child before tree removal, and add a regression that raises after temp allocation.