[TOW-1394] UV cache & package path cleanup

[sammuti]

Problems:

1. App package directory was never getting cleaned
2. When TOWER_DISABLE_UV_CACHE=true, UV created temporary cache directories and lock files in /tmp that were never cleaned up, causing GB of disk space to accumulate over time.

Solution:

• Track the app package directory and clean it explicitly
• Create isolated temp directory per execution: /tmp/tower-uv-{execution-id}/
• Pass to UV via --cache-dir flag (existing mechanism)
• Set TMPDIR env vars to same directory (captures lock files)
• Clean up entire directory after execution (normal or terminated)

Behavior:

• TOWER_DISABLE_UV_CACHE=true: Uses isolated temp cache, fully cleaned up after each run
• TOWER_DISABLE_UV_CACHE=false: Uses persistent global cache (default behavior unchanged)
• No residual files remain in /tmp after execution

[Copilot]

Pull request overview

This PR addresses a disk space accumulation issue caused by UV creating temporary cache directories and lock files in /tmp that were never cleaned up when TOWER_DISABLE_UV_CACHE=true. The solution creates isolated temporary directories per execution, configures UV to use them via both --cache-dir and TMPDIR environment variables, and ensures cleanup occurs after execution completes or is terminated.

Changes:

• Isolated temp directory management: Create unique /tmp/tower-uv-{execution-id}/ directories that are cleaned up after execution
• Environment variable handling: Modified to respect pre-set temp directory variables (TMPDIR, TEMP, TMP) to support isolated temp directories
• Test coverage: Added comprehensive tests validating temp file cleanup in happy path, termination, and cache accumulation scenarios

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
crates/tower-uv/src/lib.rs	Modified environment variable normalization to conditionally set temp vars, allowing custom temp directories to be preserved
crates/tower-runtime/src/subprocess.rs	Implemented isolated temp directory creation, TMPDIR configuration, and cleanup in both Drop implementation and explicit cleanup method
crates/tower-runtime/tests/subprocess_test.rs	Added new test suite with three tests validating temp file cleanup behavior across different execution scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test assumes the isolated temp directory uses a hardcoded path based on the execution ID, but this creates a brittle coupling between the test and implementation. If the directory naming scheme changes in subprocess.rs (line 83), this test will fail. Consider deriving the expected path from the same logic or making it discoverable rather than hardcoded.

Suggested change

	// With the fix: temp files should NOT accumulate in /tmp because they go in isolated directory
	assert_eq!(
	after_count, before_count,
	"UV temp files should NOT accumulate in /tmp with the fix. Before: {}, After: {}",
	before_count, after_count
	);
	// Verify the isolated temp directory was created
	let temp_dir_path = std::env::temp_dir().join("tower-uv-test-exec-happy");
	debug!("Checking for isolated temp directory: {:?}", temp_dir_path);
	assert!(
	temp_dir_path.exists(),
	"Isolated temp directory should exist during execution"
	);
	// With the fix: temp files should NOT accumulate in /tmp because they go in an isolated directory
	assert_eq!(
	after_count, before_count,
	"UV temp files should NOT accumulate in /tmp with the fix. Before: {}, After: {}",
	before_count, after_count
	);

[Copilot]

The magic number 10_000_000 (10MB) lacks justification for why this specific threshold was chosen. Consider defining this as a named constant with documentation explaining the rationale, or make it configurable if the acceptable growth varies by environment.

[bradhe]

Couple questions, let's get these resolved so we can get this in.

[bradhe]

Do we want to provide configuration for disabling this? It's a great default, but I feel it could cause some problems in certain cases.

[bradhe]

Ah I think I missed this--you're coupling the cache_dir and the overridden temp_dir. I think this is probably OK. It's a bit confusing at first. Here's the logic as I understand it.

1. If caching is enabled, which is the default, then we'll use the system-default temp directory.
2. If caching is disabled, we jail the temporary directory and do our own cleanup.

Why not decouple those two things from each other?

[bradhe]

Okay, I think I see that this is intentional--I think we probably will want to separate these in the future, as it's kind of surprising behavior. But we can move forward with this as-is.

[bradhe]

Why can't we have the package manage this? We're removing the package here. Seems like this logic should be encapsulated in the package?

[bradhe]

We discussed asynchronously, while it feels like we're kind of blurring the lines of the interface between the application and the filesystem here, it's not the worst thing in the world. In another follow up PR we can clean this up.