[jussielo-amd]

Summary

rocm install sdk --dry-run is meant to be a read-only preview, and the assistant's install_sdk_dry_run tool is listed among the read-only tools. On a host where no usable Python can be found (a minimal container, a fresh Windows install), it was not read-only: the preview went and prepared a managed Python before printing any plan. That meant downloading a managed uv, having uv install Python, and saving that Python's path in the CLI configuration. The preview needs a Python only to learn which wheel tags to select packages for, so none of that had to happen. Now the preview reports that a real install would prepare a managed Python, says so in a python_note: line, and prepares no Python and saves no Python path. A real install is unchanged. One qualification: the no-uv, no-tools-folder part holds for the canonical layout only, because a preview of a ROCm 10 or newer wheel resolves its package versions with uv pip compile and can still fetch the managed uv on a clean host (see the notes). Fixes ROCMAI-867.

Reproduction

The ticket described this from reading the code and had not reproduced it. I reproduced it through the end-to-end harness against a build of origin/main. The scenario serves one canonical release pip index from a loopback fixture, points PATH at a single empty folder (so no Python is found) with ROCM_CLI_PYTHON unset, and points ROCM_CLI_UV_BINARY at a recording stand-in for uv so the attempt is observable without the network. Running rocm install sdk --channel release --format wheel --family gfx1200 --dry-run on that build printed Preparing Python 3.12..., Installing Python 3.12 via uv... and Finding Python 3.12... and exited 1 (the stand-in uv finds no Python), so the dry run had called the uv python install step. The path is install_wheel_runtime in apps/rocm/src/therock.rs calling resolve_python_launcher before its dry_run return, which falls through to ensure_managed_python in apps/rocm/src/therock/python.rs. The assistant tool, the MCP tool and rocm install sdk --dry-run all reach that same function, so one fix covers them.

What I did not demonstrate: a real download of uv and Python from the internet (the stand-in uv keeps the run offline, so I observed the call to it, not bytes arriving), and a Windows run.

After the fix the same scenario, a canonical-layout preview, passes: the preview exits 0 with a plan, the recording uv is never invoked (its log does not exist, which is the evidence that nothing was prepared), no managed tools folder exists under the data folder, and no config.json was written. The scenario also looks for a tool archive in its own cache folder, but only as a backstop: with ROCM_CLI_UV_BINARY set the CLI returns the stand-in before any download could start, so that folder stays empty either way and is not the evidence.

Fix

resolve_python_launcher_in is split at the point where it decides to prepare a managed Python. The search itself (an explicit ROCM_CLI_PYTHON, the PATH search, the saved managed Python, the bootstrap-disabled refusal) moves unchanged into find_python_launcher_in, which returns either a usable interpreter or NeedsManagedBootstrap. The real resolver calls ensure_managed_python on the second answer exactly as before. A new plan_python_launcher reports it as PlannedPython::WouldPrepareManaged instead, and python_for_install is the one place the wheel install chooses between the two based on dry_run. Every error the real resolution raises before the bootstrap step is raised for the preview as well, so a preview does not print a plan on a host where the install would refuse.

That includes a managed-Python version override. ROCM_CLI_MANAGED_PYTHON_VERSION can name a version whose wheel tag is not the layout's (for example 3.13 against the canonical cp312 layout); a real install prepares that Python and then refuses it, so the preview fails with the same message (Python wheel tag cp313 is not supported by this ROCm install; cp312 is required) instead of announcing a plan. The comparison is one shared function, ensure_python_tag_matches, used both for an existing interpreter and for the version the preview would prepare. The tag is read off the version the way uv accepts it for a CPython release: 3.13, 3.13.1, the free-threaded 3.13t, and any of those after cpython- or cpython@. uv does the real parsing and this repo has no parser of its own to share, so this reader is deliberately small. A version whose tag cannot be read off its name (another implementation such as pypy@3.10, a range, a pre-release spelled on the minor) is announced as unchecked: the note and the progress line say its wheel tag cannot be checked until it is installed and what the install requires, rather than promising it.

For wheel selection the preview uses the interpreter tag the layout requires (cp312 for the canonical layout, cp314 for ROCm 10 and newer), because a real install refuses any other interpreter. The printed command shows uv venv --python 3.12 ... since the real path does not exist yet. The preview output gains python_note: only when a Python would be prepared, python_launcher: reads not prepared by this preview in that case, and python_source: is managed. Hosts that have a usable Python print exactly what they printed before.

Every progress line the preview prints about Python is derived from the same decision the install takes, so a line that says what a real install would do cannot appear on a path that ends in a refusal. The opening line, which said if needed, ROCm CLI will prepare Python X, reads Checking Python for the ROCm install. in a preview and promises nothing. The two lines about an unusable PATH Python and an unusable saved managed Python used to say a real install would use ROCm CLI's managed Python and a real install would prepare Python again in a preview, which was premature: bootstrap can be disabled, or the version override refused, after them. In a preview they now state only the finding (Python from PATH is not usable for this ROCm install (cp312 is required)., and the same for the saved Python). The single line that predicts the preparation, No usable Python found; a real install would prepare Python X. This preview does not., comes from PlannedPython::progress_text, which exists only for a decision that already passed the bootstrap policy and the version check; python_for_install prints it, together with the Using ... lines of the other outcomes, which moved there from therock.rs so the whole Python output is one value that a unit test can capture. A real install prints all of its lines exactly as before, so non-dry-run output is byte-identical.

The dash-TUI tool-group comment in crates/rocm-dash-tui/src/agent/tools.rs listed this side effect for install_sdk_dry_run; it now says update_check can still prepare a Python, and that a ROCm 10 preview can still fetch the managed uv.

Verification

Ran, on Linux with a private target dir, on the final tree: cargo fmt --all --check; cargo clippy -p rocm -p e2e-cucumber --all-targets -- -D warnings; cargo test -p rocm --bin rocm therock::; the feature_naming guard; cargo xtask manifest --check; cargo xtask check-crate-edges; cargo xtask lychee; the leak scan against origin/main...HEAD. The scenarios were run through the repo's harness with ROCM_CLI_BINARY pointing at a debug build of the branch (equivalent for these scenarios to the binary cargo xtask e2e builds), once with a leftover tool archive planted in the shared cache folder to mimic CI's reused cache. Earlier in the PR's life the whole cargo test -p rocm passed (1474 tests, against 1467 on main at the time, before the later rounds added more unit tests) and runtime-23 was run against the unfixed build, where it fails.

Mutations, each reverted by a targeted edit afterwards: making python_for_install ignore dry_run turns a_dry_run_with_no_usable_python_prepares_nothing and a_dry_run_with_an_unusable_saved_python_prepares_nothing red; making the preview save a Python path in the config turns the same two red and fails the runtime-23 Then step with the config path in the message; passing false instead of dry_run at the call site in therock.rs turns runtime-23 red at the When step with Installing Python 3.12 via uv... in the output; skipping the tag check in the preview turns a_dry_run_refuses_a_managed_python_version_the_install_would_refuse red; removing the t suffix handling, and separately the cpython- and cpython@ prefix handling, from the version reader turns both a_managed_python_version_is_read_as_the_wheel_tag_it_reports and a_dry_run_reads_every_spelling_of_a_managed_python_version_the_same_way red each time; restoring the promising opening progress line in a preview turns runtime-24 red; restoring the premature a real install would use ROCm CLI's managed Python text on the PATH line, then on the saved-Python line, and printing the prediction before the version check each turn the state-matrix test a_dry_runs_whole_python_output_follows_one_decision_in_every_state red, naming the row; asserting against the shared cache folder again, with a leftover archive planted in it, reproduces the CI failure runtime-23 once had.

Not run: clippy for x86_64-pc-windows-msvc (the aws-lc-sys build script fails to cross-compile on this host, an environment limit independent of this change; the changed non-test code has no cfg(windows) branches), the full workspace gate and scripts/smoke_local.py on the final tree, and a real network download of uv and Python.

User-visible behaviour

Scenario runtime-23 (runtime-install-preview-prepares-no-python) covers the preview on a host with no usable Python, runtime-24 (runtime-install-preview-refuses-unusable-managed-python) the version-override refusal, and runtime-25 (runtime-install-preview-refuses-disabled-managed-python) the policy refusal when the host's only Python is unusable and managed Python is disabled, all in tests/e2e-cucumber/features/runtime_setup.feature. All three are @requires-os:linux and need no GPU or network, so they run on the ordinary hermetic e2e lane. The empty-PATH host is built for the Linux search order; the decisions themselves are unit-tested on Unix hosts, and the Windows search order is not covered by a test. runtime-23's final step asserts the preview's own claim (the python_note: line and the This preview does not. progress line) together with the state it left, with the recording uv's log as the evidence nothing was prepared. runtime-25 asserts that the output holds the neutral finding about the PATH Python, the same refusal as the install, no line predicting anything, and no side effects. runtime-24 asserts the final output as a whole: the neutral opening line, none of the promises the refusal would contradict (would prepare Python 3.13, ROCm CLI will prepare, a printed plan), the refusal text on stderr, and the same absence of side effects. Both give the CLI a cache folder of their own (ROCM_CLI_CACHE_DIR under the scenario root) because the harness otherwise shares one cache across scenarios and CI runs, where a leftover archive would trip the backstop check. fake-uv gains an opt-in FAKE_UV_LOG file so a scenario can show uv was never run.

The unit test a_dry_runs_whole_python_output_follows_one_decision_in_every_state is the state matrix: every combination of PATH Python (usable, unusable, absent), saved Python (usable, unusable, absent), bootstrap policy (enabled, disabled) and version override (default, accepted patch release, unreadable, refused tag), 72 rows, each asserting the complete progress output against an independently written expectation, that no prediction line exists unless the row ends in the one promise, and that uv was not run and no file or config was written. Other boundary cases are unit-tested in apps/rocm/src/therock/python.rs too: no Python and no config, data or cache folder at all (nothing is created); a usable Python on the search path; a usable saved managed Python (the manifest and config are left untouched); an unusable saved managed Python (reported as needing a Python, record left alone); managed bootstrap disabled (preview and install refuse with the same message); a version override that the install would refuse, in the spellings 3.13, 3.13t, cpython-3.13 and cpython@3.13; patch and cpython spellings of the accepted minor; and an unreadable spelling announced as unchecked. docs/testing.md and docs/manual-testing.md describe the preview behaviour and the manual check. The --help text and README did not claim anything this change makes false, so they are unchanged.

Notes for reviewers

The preview still writes the package-index metadata cache under the cache folder (the ticket mentions it): that is an HTTP cache of the index the plan is resolved from, and docs/testing.md says so. I left it alone rather than thread a no-write mode through the resolver. A preview of a ROCm 10 or newer wheel also still calls ensure_uv_binary through resolve_published_pip_package_versions, because it resolves the published versions with uv pip compile; on a clean host that can download the managed uv and create the data folder's tools folder, although no Python is prepared. Avoiding it would mean a different resolver for the preview, which is not a small behaviour-preserving change, so docs/testing.md, docs/manual-testing.md, the scenario comments and the tool-group comment now limit the no-uv guarantee to the canonical layout. The update_check tool and rocm update --check resolve Python through the same resolve_python_launcher in therock/update.rs and can still prepare a managed Python on such a host; that is a sibling of this bug, outside what the ticket asks for, and the corrected tool-group comment says so. It is a natural follow-up using the same plan_python_launcher seam. A preview that uses an existing Python still runs that interpreter and its -m venv probe (in a temporary folder that is removed), as it always did.

Reading order: therock/python.rs (the split, PlannedPython and the version reader), then the install_wheel_runtime hunks in therock.rs, then the scenarios and their steps. Open PRs touching the same code: #563 and #568 both edit the uv_venv_args(&python_launcher.executable, ...) line in the dry-run block of install_wheel_runtime, which this change replaces with python.venv_python_arg(); the textual conflict is one line and either resolution is mechanical. #569 changes ensure_uv_venv on the real-install path only. #820 and #819 edit other parts of therock.rs and the same feature and step files, so expect trivial hunk conflicts. #901 and #812 touch agent/tools.rs elsewhere than the comment block edited here. Another scenario named runtime-22 landed on main while this was open, so these two are numbered runtime-23 and runtime-24.

[Copilot]

🟡 Changes recommended

The preview can report an incompatible managed-Python plan as valid, and the E2E cache assertion can fail because of pre-existing shared-cache contents.

2 open issues

• Reject managed Python overrides that conflict with wheel layout · New
• Use a scenario-local cache for the runtime-22 tools check · New

What changed in this PR

This PR separates Python discovery from preparation so SDK dry-run previews can report a planned managed Python without downloading tools or saving its path. Real installs retain the existing preparation flow.

Changes:

• Add preview-aware Python planning and explanatory output.
• Add unit and end-to-end regression coverage, including recorded uv invocations.
• Document preview behavior and its remaining package-index cache writes.

File	Description
tests/​e2e-cucumber/​tests/​e2e/​therock_steps.rs	Add no-Python fixtures and side-effect assertions.
tests/​e2e-cucumber/​src/​bin/​fake-uv.rs	Add optional invocation logging.
tests/​e2e-cucumber/​features/​runtime_setup.feature	Add a Linux preview regression scenario.
docs/​testing.md	Describe coverage and remaining cache writes.
docs/​manual-testing.md	Document manual preview checks.
crates/​rocm-dash-tui/​src/​agent/​tools.rs	Correct the comment describing tool side effects.
apps/​rocm/​src/​therock/​python.rs	Separate discovery from preparation and test preview decisions.
apps/​rocm/​src/​therock.rs	Use planned Python information in SDK previews.

_{Review effort: Balanced}

[jussielo-amd]

Fixed in 1936bce: the runtime-22 Given now points the cache at a per-scenario scratch folder (ROCM_CLI_CACHE_DIR), so a warm shared cache can no longer fail it; I reproduced the CI failure by planting a leftover archive in the shared cache and the fixed scenarios pass in that setup. The recording uv log is the evidence that nothing was prepared, and the cache check is a backstop (794081a).

[Copilot]

🟡 Changes recommended

Python previews claim preparation before policy and version checks pass, while testing docs omit ROCm 10 metadata resolution’s possible managed-uv download and tools-directory creation.

3 open issues

• Use a scenario-local cache for the runtime-22 tools check
• Preview messages claim managed Python before bootstrap checks pass · New
• ROCm 10 previews may bootstrap uv despite Python bootstrap being skipped · New

1 resolved since last review

• Reject managed Python overrides that conflict with wheel layout

_{Review effort: Balanced}

[jussielo-amd]

Confirmed: ROCm 10 previews call ensure_uv_binary through resolve_published_pip_package_versions to run uv pip compile, and avoiding that would need a different resolver, so it is not a small behaviour-preserving change. 72551d7 limits the no-uv / no-tools wording to the canonical layout in docs/testing.md, docs/manual-testing.md, the scenario comment, the tool-group comment and the PR body; a ROCm 10 preview still prepares no Python.

[Copilot]

🔵 Needs a closer look

A clean-host run with real uv is still needed to confirm ROCm 10 previews work without provisioning Python, since the new no-Python tests cover only the canonical layout.

2 open issues

• Use a scenario-local cache for the runtime-22 tools check
• ROCm 10 previews may bootstrap uv despite Python bootstrap being skipped

1 resolved since last review

• Preview messages claim managed Python before bootstrap checks pass

_{Review effort: Balanced}

[juhovainio]

Thanks for the work here — the Python download and config write are gone from the preview, and CI is green. Requesting changes on one scope point, plus a few smaller notes.

Needs resolution

• ROCMAI-867's acceptance says "no download of Python or uv". A ROCm 10+ preview can still fetch the managed uv (and create tools/) via uv pip compile. The PR discloses this but is titled/linked as "Fixes ROCMAI-867". Either finish the work (no uv fetch in the preview) or narrow the "Fixes" to what is actually fixed.

Non-blocking

• The doc comment on python_tag_for_version says a wildcard yields None, but the unit test asserts "3.13.*" returns cp313 (inline comment).
• The hand-rolled version reader can drift from what uv accepts. The failure is benign (the version is announced as unchecked, not refused), but worth a comment saying so.
• At ~1800 lines this is large for a fix, and the version-override refusal (runtime-24/25) goes beyond the ticket. Splitting it out would make review easier.
• Windows runs none of the dry-run decision tests (cfg(unix) tests, @requires-os:linux scenarios). Disclosed, noted for the record.
• update_check still prepares a managed Python (disclosed). docs/llm-tool-use.md and the skill text are not touched; please confirm they don't describe the old preview behaviour.
• Minor: tag_checked looks speculative, note() and progress_text() duplicate wording, and source is a bare string.

CI is all green on 72551d7.

[juhovainio]

This says a wildcard yields None, but the unit test asserts "3.13.*" -> cp313 (the patch part isn't validated). Please reword the comment to match the behavior (or tighten the parser if None was intended).