[jussielo-amd]

Summary

ROCMAI-880. With no runtime chosen, rocm comfyui install and the engine commands (rocm serve, rocm engines install) each carried their own copy of "use the one ready runtime". The copies agreed on what ready means (both end in validate_runtime_manifest_for_activation) but not on the steps around it. The engine copy first recovered the runtime rocm setup created (recover_setup_runtime_registration) and only then counted. The ComfyUI copy never recovered. So when a setup runtime's registry entry had been lost, rocm serve found the runtime and wrote the entry back, while rocm comfyui install on the same machine refused with "Install ROCm first", or, if another runtime was registered next to it, quietly installed into that one.

This moves the rule into one function, runtime_adopt::select_single_ready_runtime, next to recover_setup_runtime_registration: recover setup, load the registry, count the runtimes the activation check accepts, and return one, none (with everything registered, so a caller can say why each one does not count) or several (the ready ones). Both callers use it and the duplicate is deleted. What each caller does with "none" and "several" stays its own: the engine path still treats both as "no selection" and leaves the advice to its callers, and ComfyUI still refuses with its own /runtimes, rocm runtimes activate <key> and --runtime-id <key> text. Whether ComfyUI should also recover setup was the one real decision. It does, because the recovery only restores an entry the install tree itself still carries, the engine commands already do it, and a user who just ran setup should not see a different answer from serve and from comfyui install.

The shared newest-first runtime sort that the neighbouring ticket asks for has not landed and is not part of this change.

What moved

Symbol	Before	After
rule: recover setup, count ready runtimes, one / none / several	private copy in engine_runtime_select.rs (single_ready_runtime_key) and private copy in comfyui/runtime_env.rs (select_single_ready_runtime)	runtime_adopt::select_single_ready_runtime, result type SingleReadyRuntime, pure half classify_single_ready_runtime
engine caller	recovered, filtered, matched on ready.as_slice()	single_ready_runtime_key maps One to the key and Zero/Many to no selection
ComfyUI caller	filtered on runtime_usability_status == "ready", worded the three cases	single_ready_runtime_or_refusal words the answer only; select_default_runtime now takes paths so it can ask the rule; the configured-default branch is select_runtime_by_default_id unchanged
test fixtures	none	write_test_setup_runtime_missing_from_registry, runtime_registry_entry_names in test_support.rs

User-visible behaviour

Every difference, all on the ComfyUI side and all only on the path where nothing else says which runtime to use (no --runtime-id, no active runtime, no configured default). The engine commands behave exactly as before.

1. A ready runtime that only setup recovery can make visible is now found by rocm comfyui install, and the install uses it. Before it refused with "Install ROCm first".
2. Finding it writes the registry entry back, including under --dry-run. This is the same write rocm serve and rocm examine already make. --dry-run has no promise of leaving the registry untouched, and still prints the same install command: line; say so if you would rather the preview skip the recovery.
3. When such a recovered runtime sits beside another ready runtime, the install now refuses as ambiguous ("Multiple ROCm runtimes are ready ...") instead of silently picking the other one.
4. If the setup folder's runtime no longer passes the activation check, or its manifest is unreadable or points elsewhere, the install now fails with the recovery error ("setup runtime <key> ... is not usable") instead of ignoring the setup folder and going on. The engine commands already failed this way.

Unchanged and pinned: zero registered still says "Install ROCm first: run rocm and pick "Set up this system" ...", registered-but-unusable still lists each runtime with its reason under "No ROCm runtime is ready", several ready still names /runtimes, rocm runtimes activate <key> and --runtime-id <key> and lists the ready keys. The new tests assert each message together with the state it describes and run the remedy it names: rocm runtimes activate (as activate_runtime, its implementation) and --runtime-id both clear the several-ready refusal, and putting the interpreter back clears the none-ready refusal.

Scenarios: the changed behaviour (difference 1) had no scenario, so comfyui-15 (@id:comfyui-install-finds-the-runtime-setup-created) is new: it plants a ready runtime and a setup folder naming it but no registry entry, previews rocm comfyui install --dry-run, and asserts the install command: line names that runtime and the registry entry exists afterwards. It runs on the default no-GPU lane, and I ran it locally. comfyui-03 (comfyui-ambiguous-runtime-actionable) already covers the several-ready refusal and now reaches it through the shared function; it passes unchanged. serve-29 (serve-unrelated-live-service-still-fails-fast) is the scenario that keeps the engine path's recovery below the no-GPU refusal; the engine behaviour is unchanged and it passes. serve-31 is @requires-gpu and unchanged in behaviour; I did not run it (no GPU). Differences 2 to 4 are covered by unit tests only (comfyui_selection_recovers_the_setup_runtime_the_registry_lost, comfyui_selection_reports_a_setup_runtime_that_is_no_longer_usable, and the SetupRuntimeLostBesideAReadyOne row of comfyui_and_engine_paths_agree_on_the_single_ready_runtime); they are variations of the same recovery that comfyui-15 exercises end to end.

Verification

Ran: cargo fmt --all --check; cargo clippy -p rocm -p rocm-core --all-targets -- -D warnings (also with -p e2e-cucumber before the merge with main); cargo test -p rocm --no-fail-fast (1522 unit tests passed, 8 ignored as before; 12 added, none removed, the leaf names of every existing test unchanged); cargo xtask manifest --check; cargo xtask check-crate-edges; cargo xtask e2e -- -n "comfyui-(03|15)|serve-29" (3 scenarios passed); lychee through the commit hook; the leak scan against origin/main (only hits are the Rust word "private" in visibility wording).

Mutations, each reverted after the run:

• Readiness test in classify_single_ready_runtime changed to install_root.is_dir(): single_ready_runtime_counts_only_what_activation_accepts, engine_selection_does_not_count_an_unusable_runtime_as_ready, comfyui_selection_with_only_an_unusable_runtime_clears_once_it_is_fixed and comfyui_and_engine_paths_agree_on_the_single_ready_runtime failed.
• Recovery call dropped from select_single_ready_runtime: engine_selection_recovers_the_setup_runtime_the_registry_lost, comfyui_selection_recovers_the_setup_runtime_the_registry_lost, comfyui_selection_reports_a_setup_runtime_that_is_no_longer_usable, select_single_ready_runtime_recovers_setup_before_counting and the agreement test failed.
• A private copy re-introduced in the ComfyUI caller (classify without recovery): the ComfyUI recovery test and the agreement test failed, and comfyui-15 failed with its preview step.
• A private copy re-introduced in the engine caller: the engine recovery test and the agreement test failed. (The agreement test gives each path its own copy of the registry state; an earlier version shared one, so a recovery by the first path hid the other's missing recovery, and this mutation is what showed it.)

Not run: the Windows cross-check cargo clippy --target x86_64-pc-windows-msvc could not run here (the C toolchain aws-lc-sys needs is missing); nothing in this change is cfg(windows), and comfyui-15 is portable but is not in the Windows lane's selected ids. The full workspace gate, scripts/smoke_local.py, and the GPU-only scenarios are left to CI.

Notes for reviewers

Read the commits in order. The first only adds tests and passes on the unmodified code: it pins both paths for zero, one and several ready runtimes, the lost-setup-runtime case, and each message together with its state and remedy. The second makes the change, flips the ComfyUI recovery test to the new behaviour, adds the agreement test and comfyui-15, and updates docs/architecture.md and docs/testing.md. The third pins difference 4. The fourth merges main (the only conflict was one line of docs/architecture.md).

Six existing ComfyUI tests called select_default_runtime or select_single_ready_runtime with in-memory manifests; they keep their names and assertions, and now call single_ready_runtime_or_refusal(classify_single_ready_runtime(..)) or select_runtime_by_default_id.

Other open work that touches the same files: #922 (one rule for finding a Python environment root from its interpreter) also edits apps/rocm/src/comfyui/runtime_env.rs, apps/rocm/src/runtime_adopt.rs, docs/architecture.md and docs/testing.md. This change does not touch the environment-root functions it edits, so I expect textual conflicts in the same files (the runtime_env.rs and runtime_adopt.rs test modules, and the docs/architecture.md entries for both modules) but not a logical one; whichever lands second rebases.

[Copilot]

🟡 Changes recommended

Dry-run installation now restores a missing registry JSON file despite promising no file changes, so recovery must avoid writes during previews or the help and documentation must disclose the exception.

1 open issue

• Dry-run install can persist registry recovery despite its documented promise · New

What changed in this PR

Unifies automatic runtime selection for ComfyUI and engine commands. ComfyUI now recovers a setup-created runtime’s missing registry entry before deciding whether exactly one runtime is ready.

Changes:

• Centralizes recovery and readiness classification while preserving caller-specific selection and refusal behavior.
• Adds regression tests, shared fixtures, and a no-GPU scenario for ComfyUI setup-runtime recovery.
• Documents the shared selection rule and scenario.

File	Description
tests/​e2e-cucumber/​tests/​e2e/​comfyui_steps.rs	Adds recovery fixtures and preview/registry assertions.
tests/​e2e-cucumber/​features/​comfyui.feature	Adds the setup-runtime recovery scenario.
docs/​testing.md	Documents how to run the recovery scenario.
docs/​architecture.md	Describes the shared rule and its callers.
apps/​rocm/​src/​test_support.rs	Adds lost-registration fixtures and registry inspection helpers.
apps/​rocm/​src/​runtime_adopt.rs	Implements shared recovery and readiness classification.
apps/​rocm/​src/​engines_cmd.rs	Tests ambiguous-selection remedies.
apps/​rocm/​src/​engine_runtime_select.rs	Delegates selection to the shared rule and adds regression tests.
apps/​rocm/​src/​comfyui/​runtime_env.rs	Uses shared selection while retaining ComfyUI refusals and testing recovery outcomes.

_{Review effort: Balanced}

[jussielo-amd]

Superseded by #934: this pull request's commits are included in that branch (head 6f043a8), and #934's description covers this change. Closing so the review happens in one place.