fix(uv): colocate the uv cache with the ROCm CLI data dir

[rominf]

Summary

Point uv at a cache inside the ROCm CLI data directory, so the cache sits on the same filesystem as the environments it populates and hardlinking keeps working.

Root cause

uv_command_env() set only UV_HTTP_TIMEOUT, never UV_CACHE_DIR, so the cache stayed at $HOME/.cache/uv while managed environments were created under the data directory. When those are on different filesystems, uv cannot hardlink and falls back to copying every file — each environment then carries its own full copy of the ROCm SDK and the torch stack.

This is not an exotic setup. It happens with a split /home, a container overlay, or a user who moved the data directory to a larger disk with ROCM_CLI_DATA_DIR or --prefix — so the workaround for running out of space multiplied the space used.

Measured with uv 0.9.30 on a host where $HOME/.cache is a separate mount, installing a small package and comparing inodes:

same filesystem  -> inode=1740514 links=2 dev=830   (hardlink into the cache)
different fs     -> inode=1740556 links=1 dev=66    (full copy)

A distinct inode with link count 1 — a copy, not a reflink or symlink. Setting UV_LINK_MODE=hardlink explicitly does not help: the request is downgraded and the command still exits 0.

uv does print a fallback warning, but two of the three install paths discard it — ComfyUI pipes stderr to a log file, and the vLLM install captures output with .output() and drops stderr on success.

Technical decisions

Keyed off data_dir, not cache_dir. A cache would naturally live in the cache directory, but ROCM_CLI_CACHE_DIR can point at a different filesystem from the environments, which would reintroduce the exact bug. Colocation with the environments is the property that matters here. Worth a maintainer sanity-check that this matches the intended meaning of the two directories.

An existing UV_CACHE_DIR wins. If one is already set, no pair is emitted and the child inherits the user's choice. This also keeps the e2e harness working, which sets UV_CACHE_DIR itself to share one cache across scenarios.

Threading &AppPaths through uv_command_env touched the call sites in therock.rs, comfyui.rs, and the vLLM engine; all of them already had paths in scope, so no new AppPaths::discover() calls were introduced.

Consequences worth noting

• rocm uninstall now covers the uv cache as a side effect, since it removes the data directory. That reclaims more space but makes uninstall slower.
• The cache is no longer shared with other uv projects on the machine. Users who want the old sharing can set UV_CACHE_DIR themselves; that escape hatch is deliberate but is not yet documented anywhere.
• Existing installs re-download once. The old $HOME/.cache/uv contents are not migrated and nothing removes them. A migration or a one-line notice may be wanted — not included here.

Tests

Three unit tests: the cache path is derived from the data dir (with UV_HTTP_TIMEOUT still present), an explicit UV_CACHE_DIR wins, and the derived path moves when the data dir moves.

They exercise the pure helper rather than mutating the process environment, because the crate denies unsafe_code and std::env::set_var is unsafe in this edition. That also keeps them parallel-safe.

CI note

cargo test --workspace on this branch shows two failures in proc_lifecycle that are unrelated to it — see #168, fixed by #169. Everything else passes; cargo fmt and cargo clippy --workspace --all-targets -- -D warnings are clean.

Fixes #160

[rominf]

The windows-build-and-test failure here is the known lifecycle-windows-http-install flake tracked in #173, not this change.

Failure signature on every attempt, unchanged:

rocm-cli installer: failed to download
http://127.0.0.1:<port>/rocm-cli-windows-amd64.zip:
An error occurred while sending the request.
FAIL: 'lifecycle-windows-http-install' was expected to pass on this host but FAILED

That is install.ps1 fetching from the test's local HTTP server, before any rocm code executes. This branch changes only Rust — uv.rs, runtime.rs, therock.rs, comfyui.rs, and the vLLM engine — and touches neither the installer nor packaging, so it cannot reach that code path.

The same scenario failed and then passed on re-run, with no code change, on #166, #169 and #172 today. The per-PR failure count rose through the afternoon (1, then 2, then 3, then 5 here), which looks like a worsening runner or network condition rather than anything branch-specific.

I have re-run this job five times and stopped there rather than keep consuming runner time. Could a maintainer re-run it, or advise if the Windows lane is known to be unhealthy right now? Everything else on this PR is green, and fmt, clippy -D warnings, and the test suite pass locally.

[volen-silo]

Reviewed at e10c649. The core mechanism is right and the execution is careful — root cause identified at the correct layer, measured evidence rather than assertion, every uv spawn site found and threaded (I checked all six independently), and a test seam that works around #![deny(unsafe_code)] making set_var unusable. The PR body also self-discloses its known gaps, which made this much faster to review. Two blocking items, one of them a two-minute edit.

Blocking

1. --prefix installs are not covered — and that's the mainline flow

crates/rocm-core/src/uv.rs:78 derives the cache from paths.data_dir unconditionally. But at apps/rocm/src/therock.rs:812:

let install_root = prefix.unwrap_or_else(|| managed_runtime_root(paths, "wheel", &runtime_key));

With --prefix, install_root is a raw user path and data_dir is untouched. The two are never cross-checked, so the same-filesystem property this PR exists to establish does not hold for prefixed installs.

This isn't an edge case:

• The built-in assistant's system prompt (apps/rocm/src/main.rs:8439) instructs the model to always obtain an install folder and pass --prefix, and forbids proposing an install command without one. Every assistant-driven SDK install goes down this path.
• It cascades to ComfyUI — apps/rocm/src/comfyui.rs:666 roots the app at install_root.join("apps").join(APP_ID), so the uv install at comfyui.rs:1426 inherits the prefixed root. Two of the three uv install paths are affected.
• One combination is a genuine regression. ROCM_CLI_DATA_DIR=/mnt/big plus --prefix $HOME/envs: before this PR the cache was ~/.cache/uv, same filesystem as $HOME/envs, hardlinks worked. After, the cache is /mnt/big/uv-cache and uv falls back to copying. The two knobs are independent and nothing validates one against the other.

There's also an existing convention pointing the other way. docs/testing.md:161-164 specifies a localized cache inside the selected ROCm runtime folder at <install-root>/pip-cache, "including both generated managed folders and explicit --prefix folders" — and docs/manual-testing.md:41-46 says the same in user terms. The documented intent is colocate-with-install-root, which covers --prefix; this PR colocates with data_dir, which doesn't. Either follow that convention or say why it's being changed.

The fix looks cheap, which is why I'm flagging it rather than deferring. The target root is already in scope at four of the five call sites: ensure_uv_venv (therock.rs:2218) takes install_root; run_uv_progress_command's only caller (therock.rs:922) has it as a local; run_uv_logged_command's only caller (comfyui.rs:350) has runtime.python; install_vllm_with_uv (engines/vllm/src/lib.rs:926) takes python: &Path. A uv_command_env_for_target(paths, target_root) is a signature change at four sites. ensure_managed_python (therock.rs:2747, :2760) genuinely doesn't fit — it installs an interpreter into uv's own global store — and should stay keyed off data_dir.

If you'd rather land the improvement now and handle --prefix separately, that's a reasonable call — but then it needs to be an explicit non-goal in the description with a follow-up issue, not silently implied as covered. Which brings me to:

2. The description and commit message state something factually wrong about --prefix

PR body and commit e10c649 both say:

a user who moved the data directory to a larger disk with ROCM_CLI_DATA_DIR or --prefix

--prefix does not move the data directory. It relocates install_root for a single SDK install and leaves paths.data_dir — and therefore the uv cache — exactly where it was. Listing it among the fixed scenarios tells reviewers a case is handled when it isn't, and it's precisely the sentence that would stop the next reader from noticing finding 1. Free to correct in the body; the commit message needs an amend or a follow-up note.

Non-blocking

• apps/rocm/src/main.rs:438-442 — the uninstall flag help is now misleading. --keep-data says "Keep app data such as logs, services, and engines" and --keep-cache says "Keep caches." build_uninstall_plan buckets strictly by paths.data_dir / paths.cache_dir, so after this PR --keep-cache has zero effect on the uv cache while --keep-data silently preserves what will often be the largest directory the product manages. A one-line amendment to --keep-data is close to required. (The broader data-vs-cache taxonomy issue pre-dates this PR — managed_pip_cache_dir was already under the data root — so re-bucketing is a separate concern.)
• crates/rocm-core/src/uv.rs:78-81 — the behavior the PR actually adds is untested. All three new tests call the private uv_command_env_with_inherited_cache with a pre-resolved Option<&OsStr>, so the env read and the .filter(|v| !v.is_empty()) are never exercised in either direction. Avoiding set_var is the right call, but the empty-string boundary is a pure function and could be tested as one.
• crates/rocm-core/src/uv.rs:440-448 — command_env_cache_dir_follows_a_relocated_data_dir is near-tautological. managed_uv_cache_dir is an unbranched root.join("uv-cache"), so building two AppPaths with different data_dirs and asserting the results differ only fails if the implementation ignores paths entirely, which the preceding test already covers. It doesn't touch AppPaths::with_managed_root or ROCM_CLI_DATA_DIR, so the name overstates it.
• Stale $HOME/.cache/uv — you flag this as known. Worth noting there's house precedent for the light version: maybe_migrate_legacy_dashboard_config (main.rs:924-939) does one-shot legacy-location detection and prints a one-line notice. A notice would fit; a full cache migration seems neither needed nor advisable.
• crates/rocm-core/src/uv.rs:65 — the escape hatch piggybacks on a third-party variable, against local convention. Every neighbouring override in this same module is namespaced: ROCM_CLI_UV_BINARY, ROCM_CLI_UV_VERSION, ROCM_CLI_UV_TIMEOUT_SECS. UV_CACHE_DIR is uv's generic variable, so a developer who has it exported for unrelated Python work silently opts out of the fix with no signal — indistinguishable from an accident. A ROCM_CLI_UV_CACHE_DIR (keeping UV_CACHE_DIR inheritance for the e2e harness) would match the convention and let the CLI report that an override is active.
• MANIFEST.md:666-672 — the uv section discloses that the uv binary is cached in the managed data directory but not the multi-GB wheel cache that now lives there. For a disclosure document that materially understates the footprint.

While you're here (pre-existing, but this PR is the natural place)

• apps/rocm/src/main.rs:9411, :9423 render Downloads/cache: <value> from active_runtime_pip_cache_dir / setup_runtime_pip_cache_dir, which fall back to <install_root>/pip-cache. But nothing anywhere sets PIP_CACHE_DIR or --cache-dir, and every manifest constructor writes pip_cache_dir: None. So the CLI tells the user downloads stay in their chosen ROCm folder while they actually go to the uv cache. Already wrong before this PR — but this is the PR that decides where downloads live, and fixing finding 1 by keying off install_root would make the existing message true.
• apps/rocm/src/therock.rs:3409 — managed_uv_cache_defaults_inside_generated_runtime_folder only asserts install_root.starts_with(&paths.data_dir) and never touches managed_uv_cache_dir or uv_command_env. It predates this PR, but its name now claims coverage of exactly what changed here. Repurposing it into a --prefix divergence test would have caught finding 1.

Nit

crates/rocm-core/src/uv.rs:79 — the inherited-value check doesn't trim, while both siblings in the same file do (uv_version at :225, env_secs at :332). UV_CACHE_DIR=" " is treated as a deliberate override.

On the data_dir-over-cache_dir question you raised

Your reasoning holds. ROCM_CLI_CACHE_DIR can point at a different filesystem from the environments, which would reintroduce the bug, and AppPaths::with_managed_root already derives cache_dir from data_dir by default. The right axis to colocate on is the one the environments live on — which is also the argument for install_root in finding 1.