[rominf]

Fixes #533.

When the Python in an install sdk folder no longer ran, or no longer matched the required interpreter, the wheel install rebuilt the environment by deleting the whole folder. With --prefix, that folder is whatever the user named, so a home directory with a failing ~/bin/python was deleted, and only then did the rebuild fail. Even a folder ROCm CLI created holds more than the environment: ComfyUI keeps the user's models, workflows and outputs under apps/comfyui.

Now:

• Reuse only a real venv. An existing environment is reused only when it runs, matches the required interpreter, has pyvenv.cfg, and its Python's own sys.prefix is the folder. Without that last check, a venv whose pyvenv.cfg was lost leaves bin/python behaving as the base interpreter, and the package install would go into the base Python (for example a pyenv or conda base).

• A missing or empty folder gets a new venv.

• Any other non-empty folder goes through one check, whether or not its Python is still there. Without it, a partly cleared venv handed to a plain uv venv is deleted whole by uv 0.7, built over with stale packages by 0.9, and refused for good by 0.10.

• That check refuses:

  • a protected system location, judged both as written and by where it resolves;
  • a folder with no record of a finished ROCm CLI install. Proof is either the in-tree .rocm-cli-runtime.json from a wheel install naming this folder, or a location inside the managed runtimes folder (which covers an install that failed before writing its manifest).

  The refusal names the folder and what was found, changes nothing, and says to choose an empty folder or one where a ROCm CLI install finished.

• For an owned folder, only the venv's own entries are removed, in this order: bin/Scripts, lib/lib64/Lib, include/Include, CACHEDIR.TAG, and pyvenv.cfg last. A clear that stops part-way therefore never leaves something that looks like a working venv. Links are unlinked, not followed. It then runs uv venv --allow-existing and prints what it kept, read from the folder after clearing. The next run finishes a clear that stopped part-way.

• uv venv always runs with UV_VENV_CLEAR=0, and never without --allow-existing over a non-empty folder.

• --dry-run reports which of these will happen. For a refused folder it prints command: none; for a reusable venv it shows only the uv pip command. To predict reuse it runs the folder's existing Python (--version, its wheel tag and sys.prefix), as the real install does; it changes nothing.

Tradeoff: a wheel install's marker counts as ownership even if the venv itself is gone, so in that folder anything the user later put under bin, lib or include is cleared. Requiring pyvenv.cfg instead would make a part-way clear unrecoverable.

Tests

• Unit tests (Unix). Each guard was mutation-checked. They cover:

  • refusal of an unowned folder, with the tree left byte-identical, and the refusal's advice getting past it;
  • an owned rebuild keeping ComfyUI data, asserted together with the exact printed line;
  • every point a clear can stop at, converging on the next run;
  • a Python without pyvenv.cfg, or with a foreign sys.prefix, never being reused;
  • markers naming another folder, tarball markers, a manifest-less managed venv, a linked lib64, and protected paths by spelling and by resolution;
  • the order a clear removes entries in;
  • the dry-run lines for every folder state;
  • a literal 56-row table of folder shapes, with expected outcomes and user paths written out, including folders holding only a README, .DS_Store or .git.

• An opt-in test (ROCM_CLI_TEST_UV_BINARIES, ROCM_CLI_TEST_PYTHON) runs every clear stop point against real uv 0.7.19, 0.9.30 and 0.10.0, and passes on all three.

• New Gherkin scenarios run the real CLI with no GPU or network:

  • @id:runtime-install-sdk-refuses-unowned-prefix (runtime-23): the dry run prints command: none, the install refuses, and the advised empty folder works.
  • @id:runtime-install-sdk-rebuild-keeps-comfyui-data (runtime-24).

  They use a loopback index, a ROCm 10 pin with an exact arch, and a fake uv that, like uv 0.10, refuses a non-empty folder without --allow-existing and wipes it on --clear. They are @requires-os:linux because the fakes are shell scripts, so Windows has no e2e coverage of this path.

Notes

• Home itself is not on the protected-location list on main, so a home folder is refused by the ownership check. fix(uninstall): refuse system folders and homes, require ROCm CLI's marker elsewhere, and warn when a shared cache will go (ROCMAI-465) #569 extends that list.
• comfyui install --reinstall's handling of apps/comfyui/source is fixed separately in fix(comfyui): reinstall replaces only ComfyUI's code, never the user's files #546.
• README, the --prefix help and docs/manual-testing.md are updated.

[jussielo-amd]

Thanks for the careful work here: refusing to delete a folder that isn't provably ours, and clearing only the venv's own entries in a crash-safe order, is the right fix. I reviewed the code only and did not run the tests.

Two blockers (inline):

1. Windows test build is red: two test-only imports are unused on Windows under -D warnings.
2. --dry-run now executes the prefix's bin/python, and install_sdk_dry_run is a model-callable, no-approval tool with a model-chosen prefix, so a preview can run an untrusted binary.

Non-blocking, in short (some inline):

• The refusal text blames "does not run or does not match" even when the cause is a missing pyvenv.cfg or foreign sys.prefix.
• The dry-run command: line omits the venv-file clearing and the UV_VENV_CLEAR=0 pin, so pasting it doesn't reproduce the install; the refusal text is also repeated after "the install will refuse this folder:".
• docs/manual-testing.md still says --dry-run "never prompts or refuses" right above the new bullet saying it predicts a refusal; the README's "managed runtimes folder" is narrower in code (<data>/runtimes/wheel).
• The branch also conflicts with main in apps/rocm/src/main.rs (the --prefix doc comment), so it needs a rebase.
• The lychee failure looks like a download problem on the runner, not this PR.

---

🤖 by agenthub on AMD AgentHub

[jussielo-amd]

Blocker (red CI): these two imports (managed_runtime_root, PROGRESS_LINE_SINK) are only used by #[cfg(unix)] tests, so on Windows they are unused and -D warnings fails the test build ("Windows build and test (debug)" / windows-build-and-test). Gating the two use lines with #[cfg(unix)] (or importing them inside the unix tests) should fix it.

[jussielo-amd]

Blocker (security): --dry-run now executes <prefix>/bin/python (--version, the sys.prefix probe and the wheel-tag probe) to predict reuse. install_sdk_dry_run is a model-callable tool treated as read-only, so it runs without approval, and its --prefix is model-chosen (only system folders are rejected in internal_mcp_install_sdk_args). A folder containing a hostile bin/python (e.g. a downloaded/cloned tree) would therefore be run with the user's rights from a "preview". The real install is approval-gated, the preview isn't. Suggest keeping the dry run static (check that the Python, pyvenv.cfg and the marker exist and say "would reuse if it runs"), or skipping the exec on the model-callable path.

[jussielo-amd]

Non-blocking (UX): whenever bin/python exists the refusal says it "does not run or does not match". But a Python that runs fine and simply has no pyvenv.cfg, or reports a different sys.prefix (e.g. --prefix pointing at a conda env), is refused too, and this wording is then misleading. Naming the failed condition ("no pyvenv.cfg", "its sys.prefix is X", "version mismatch") would let the user act on it.

[jussielo-amd]

Non-blocking: symlink_metadata failures of any kind (not just NotFound) return Ok(()) here, so a permission or I/O error is reported as a successful clear and uv venv --allow-existing then runs over a stale entry. Only swallowing ErrorKind::NotFound would keep the "never leaves a half-cleared venv" promise honest.