fix(comfyui): pin managed torch stack during install (EAI-8051) - #298
Conversation
ComfyUI dependency install runs `uv pip install` into the machine's managed ROCm runtime. Direct torch/torchvision/torchaudio specs are filtered out, but nothing scopes resolution, so a transitive dependency still resolves torch from PyPI and drags in a CUDA build plus nvidia-* wheels, displacing the runtime's ROCm torch and leaving a runtime with no AMD GPU support. Probe the runtime's installed torch-stack versions via importlib metadata (no native import) and pass a uv --constraint file pinning them to those exact versions. The resolver treats them as satisfied and never swaps in a CUDA build; ROCm torch has no nvidia-* deps, so none appear. A genuine version conflict now fails the install loudly instead of silently corrupting the runtime. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
r0x0r
left a comment
There was a problem hiding this comment.
Reviewed — the approach is sound. Pinning the runtime's torch stack to its exact installed versions (local +git… identifiers and all) via a uv --constraint file is the right lever: a transitive dep that wants a compatible torch is satisfied by what's already there (no reinstall), and a genuinely incompatible one fails loudly instead of silently swapping in a CUDA build. I checked the TORCH_STACK_VERSION_PROBE string — the Rust \-line-continuations strip the source indentation and the \x20 escapes re-supply it, so the emitted Python has valid, consistent block indentation. The pure helpers are unit-tested, and the end-to-end CUDA-rejection proof is the @requires-gpu @nightly comfyui-install-preserves-the-rocm-runtime scenario in #291 — a gated lane, named per AGENTS §3.
One substantive thread on the scope of the guarantee below.
Address review: make explicit why constraining only the installed torch-stack packages is safe. A managed runtime installs torch/torchvision/torchaudio atomically and refuses unless a mutually compatible set is found, and ComfyUI install only targets such a runtime — so the "torch present, torchvision/torchaudio absent" case in which an unpinned CUDA torchvision could slip in alongside the pinned ROCm torch cannot occur. Comment-only; no behavior change. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
…note (EAI-8051) Review nit: the resolver is `select_matching_pip_package_versions`, not `select_compatible_stack`. Comment-only. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
r0x0r
left a comment
There was a problem hiding this comment.
Approving. The transitive-torch defect (EAI-8051) is fixed at the right layer: the managed runtime's exact torch/torchvision/torchaudio versions are pinned via a uv --constraint file, so the resolver treats the ROCm stack as satisfied and can never swap in a CUDA build, while a genuine conflict fails loudly instead of silently corrupting the runtime. The \x20-escaped Python probe reconstructs to valid indentation, and the pure helpers are unit-tested. My one thread -- whether "constrain only what's installed" leaves a partial-stack gap -- is resolved: the follow-up commits document (and I verified) that therock installs the full trio atomically as a mutually-compatible set (select_matching_pip_package_versions bails otherwise) and ComfyUI's select_runtime only ever targets a managed wheel runtime, so the partial-stack case cannot arise. End-to-end CUDA-rejection proof is the @requires-gpu @nightly scenario in #291, named per AGENTS. 24/24 CI green.
The ComfyUI fix landed in 3e59435 (#298), pinning the managed runtime's torch stack through uv constraints. The scenario now asserts behavior main implements; remove its xfail row so a pass is not a fatal stale XPASS. Keep the scenario as the regression guard. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
main's #305 (f2c3477) already fixed EAI-8053 — the driver install no longer prepends `sudo` when it is already running as root — and shipped unit tests over the install plan covering both the root and non-root cases. The e2e scenario this branch carried is therefore redundant, and no CI lane runs as uid 0, so it could never have executed anyway. Remove the scenario, its `@requires-root` tag, the `is_root` capability probe, the `mock-root` fixture and the expectations row. What remains is the EAI-8051 ComfyUI contract: - Register `comfyui.feature` in `FEATURE_KEYS` and renumber its scenario to `comfyui-01`, per the per-feature naming guard main added. - Require the install to exit 0 before asserting runtime health. Since #298 it does; without that premise a bail-out (no managed runtime, a failed download) would leave the runtime trivially unchanged and the scenario would report green having installed nothing. - Correct the now-stale comments claiming the install exits non-zero, and describe `find_venv_python` accurately: the documented layout is the first frontier entry, so the depth-capped walk is a fallback, not a cost the normal path pays. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
Summary
rocm comfyui installinstalls ComfyUI's dependencies into the machine's managed ROCm runtime. It already filterstorch/torchvision/torchaudioout of ComfyUI's direct requirement specs, but nothing scopes package resolution — so a transitive ComfyUI dependency can still resolvetorchfrom PyPI, pull a CUDA build plusnvidia-*wheels, and displace the runtime's ROCm torch. The runtime the whole machine serves models with then becomes a CUDA build with no AMD GPU support: installing an optional app breaks the base runtime (and vLLM serving with it).This pins the managed runtime's existing torch stack to its exact installed versions so the resolver can never swap in a CUDA build.
Changes
uv pip installfor ComfyUI's deps, probe the managed runtime's currently-installedtorch/torchvision/torchaudioversions viaimportlib.metadata(no native import needed).--constraintfile and pass it to the install. The resolver treats the stack as already satisfied and never replaces it; because ROCm torch has nonvidia-*dependencies, none are pulled in.--constraintargument wiring.Test plan
cargo test -p rocm(Linux) — 477 tests pass, including 3 new ComfyUI tests.cargo clippy/cargo fmt --checkclean.@requires-gpu @nightlyscenariocomfyui-install-preserves-the-rocm-runtimefrom test(e2e): pin the ComfyUI runtime-preservation contract (EAI-8071) #291 (EAI-8071), which runs on a GPU host after that contract lands.