[volen-silo]

Problem

therock-next-09 ("the user reinstalls vllm") fails on every run of E2E tests (MI300X, current), on main as well as on PRs. It is the one unexpected failure in that lane; the other failures there are expected-failure entries in tests/e2e-cucumber/expectations.toml.

ROCm 10.x vLLM discovery pins amd-aiter to the newest release of its series (0.1.24.post1). The newest vLLM 0.29 wheel on the staging index depends on an exact older build, amd-aiter==0.1.22.post2.dev0+..., so uv reports the pair as unsatisfiable and the install fails.

Change

After the vLLM pin is discovered, the install asks uv what that exact vLLM depends on and takes the flash-attn and amd-aiter pins from the answer. This adds these uv calls, all --dry-run and none installing anything:

• one resolve probe for the discovered vLLM (with the same config file the real install uses);
• one confirm call per companion pin the probe named, against the vLLM index alone, so a build that only exists on PyPI never becomes the pin. It is tried twice, so a single transient failure does not land on the series pin, which on ROCm 10.1 is the pair that cannot be installed together;
• when the confirm call fails both times, the series-match call that was used before this change.

The probe and the confirmation only improve on the series match, so neither can make an install fail that would have worked without them:

• If the probe fails, both companions use the series match.
• If a confirm call fails (an index that does not hold the build, or a transient network error), that companion uses the series match.
• Each fallback is reported in the install warnings with the reason. If the real install then fails with those fallback pins, the same reasons are appended to the error after the install's own failure, so the note is not lost on the path where it matters and plain display still leads with the uv failure.

The probe uses the same default index and config file as the real install but not the same flags (--reinstall for the whole set, no torch pins), so it approximates that install and does not guarantee it; only the companion lines of its output are used.

docs/vllm.md describes this.

Verification

• Reproduced the resolver failure in WSL2: the same uv command with the CI argv fails with the same message, and with the new pins it resolves.
• Unit tests in engines/vllm/src/install.rs use a fake uv (cfg(unix)). Two helper-level tests cover the companion-pin choice, including the fallback note. Four tests drive the real resolve output through install_vllm_rocm10_discover or the companion helper: discovery_installs_the_companions_the_discovered_vllm_depends_on (the 0.1.22 build reaches the real install), a_failed_resolve_probe_falls_back_to_the_series_and_says_why, and an_unconfirmed_companion_pin_is_reported_and_follows_a_failed_install (the reason appears in the warnings, and in the error when the install fails). a_transient_confirm_failure_is_retried_once_before_falling_back uses a fake uv that fails the first confirm and succeeds the second. I made a failed probe fatal again, removed the retry, and put the note back in front of the cause; the tests failed each time, then I restored the change.
• An existing test that asserts the exact list of uv dry-run calls (install_vllm_rocm10_discover_realigns_torch_after_the_config_file_install) needed one new expected entry for the probe; its torchaudio assertion is unchanged.
• Under WSL2 Ubuntu: cargo fmt --check, cargo clippy -p rocm-engine-vllm --all-targets -- -D warnings and cargo test -p rocm-engine-vllm --all-targets (128 tests) pass.
• Not run here: scripts/smoke_local.py. The new tests are cfg(unix) because the fake uv is a shell script, so the Windows lane does not run them; the logic they cover is not platform-specific.

Scenario coverage (AGENTS.md section 3)

The user-observable behaviour is that the vLLM reinstall on a ROCm 10.x preview runtime succeeds and reports its discovery pins. The scenario that covers it is @id:therock-next-09-live-install-reports-vllm-rocm10x-discovery-pins, tagged @requires-gpu @requires-engine:vllm, so it runs only on the gated GPU lane (MI300X). It fails on main on every run of that lane with the amd-aiter error above. Whether it passes with this change is what this PR's MI300X run will show; that run is still queued, so it is not yet proven. Locally the scenario stopped on the OpenMPI prerequisite, because sudo needs a password on the test host.

Docs

docs/vllm.md is updated. docs/testing.md and docs/manual-testing.md are deliberately unchanged: no command, flag, exit code or output format changes. The change is which pins the install chooses, plus a warning when it falls back.

[juhovainio]

I went through this against the diff. The core idea is sound: taking flash-attn and amd-aiter from the resolver's answer for the exact vLLM fixes the incompatible pair, and confirming the version against the vLLM index only keeps PyPI builds from becoming the pin. I found nothing that breaks the fix itself, but a few things should be tightened before merge, so I'm requesting changes.

1. A failed resolve probe aborts the install, but docs/vllm.md and the PR description both say the series match decides otherwise. The fallback only covers "resolved pin not on the vLLM index", not "probe failed". Either fall back to the series pins on error (with a logged reason) or say plainly in the docs that a failed probe fails the install.
2. The error from the exact-version confirm call is discarded, so a transient uv or network failure silently reverts to the series pin, which is the pair this PR is fixing. At minimum log the dropped error.
3. The new unit tests only exercise the companion-pin helper with a hand-written resolver string, and both are cfg(unix). Nothing covers the real resolve output flowing through install_vllm_rocm10_discover. Please say in the PR text that @id:therock-next-09-live-install-reports-vllm-rocm10x-discovery-pins is the scenario that covers this, that it fails before and passes after, and that it runs on the gated GPU lane. Its MI300X run is still pending.
4. The description says the change adds "one uv call". It adds a resolve probe, a confirm call per companion pin, and a fallback call when the confirm misses. Worth correcting.
5. docs/testing.md and docs/manual-testing.md are untouched. If that's deliberate, a line in the PR text saying why would help.

One concern I chased and dropped: uv pip install --dry-run --reinstall still prints the + pkg==ver line for an already-installed package, so the parser works for reinstalls.

CI has no failures so far, but the E2E lanes are still pending.

[juhovainio]

resolve_vllm_dependencies(...)? makes a failed probe fatal, whereas docs/vllm.md says the series match decides otherwise. Consider falling back to the series pins on Err (with a logged reason), or document that a failed probe fails the install.

[juhovainio]

The Err from the exact-version confirm is discarded by let Ok(pin), so a transient uv or network failure silently reverts to the series pin, i.e. the pair this PR is fixing. Worth logging the dropped error.

[juhovainio]

This says the series match decides otherwise, but a failed resolve probe aborts the install instead (see install_vllm_rocm10_discover). Either make the code match this sentence or mention the abort here.

[nowycondro]

🔴 Automated review · pr-review-watcher · 61a1883

Review — no blocking findings

Full review of the whole change (engines/vllm/src/install.rs, docs/vllm.md) against merge-base efb9bc3. No ticket named in the PR description; nothing to reconcile against a tracker.

Blocking

None

Non-blocking

• The confirmatory per-package resolve error is discarded before falling back to the series pin — engines/vllm/src/install.rs:779-788 (discover_vllm_companion_pin)
  The if let Some(requirement) = dry_run_resolved_pin(...) && let Ok(pin) = discover_pinned_requirement_for(...) chain silently drops the Err from the confirm call and falls through to the series match on any failure, not only "the vLLM index doesn't serve this exact build" (the intended case, and the one the doc comment and the second new unit test cover). A transient uv/network failure on the confirm call is indistinguishable from that case and is never surfaced; if the series fallback then also fails, the user only sees the fallback's error message, with no trace that the exact-pin attempt was made or why it failed.
  Confidence 70 · logic · Fix: log the discarded error (e.g. at debug/info level) before falling back, so a genuine infra failure is distinguishable from "not served" during triage.
• No test exercises the "companion pin wins" branch through the real install_vllm_rocm10_discover orchestration path — engines/vllm/src/install.rs:2045-2076 (new test companion_pins_follow_what_the_discovered_vllm_depends_on), engines/vllm/src/install.rs:1065 (install_vllm_rocm10_discover)
  The two new unit tests call discover_vllm_companion_pin directly with a hand-written resolution string, which is a reasonable unit test of that function, but the modified integration-style test (install_vllm_rocm10_discover_realigns_torch_after_the_config_file_install) uses a fake uv that only ever echoes back the requested package's own name, so it only ever exercises the fallback branch end-to-end. The "take the pin from vLLM's own resolution" branch is never exercised through the full discovery/install function that threads config_file/vllm_resolution together.
  Confidence 70 · test-coverage · Fix: add (or extend) an integration-style test whose fake uv names flash-attn/amd-aiter in the vLLM resolve's dry-run output, asserting install_vllm_rocm10_discover's returned pins take that answer.
• docs/vllm.md doesn't say what happens if the vLLM dependency-resolve probe itself fails (as opposed to succeeding but not serving the exact pin) — docs/vllm.md:112-122
  The new paragraph says "the install resolves that exact vLLM with uv pip install --dry-run ... Otherwise the series match above decides, as before." A reader can take "otherwise" to cover every case other than a successful companion match, but in the code the resolve call itself (resolve_vllm_dependencies, engines/vllm/src/install.rs:721-757) has no fallback on failure — its ? aborts the whole install before the series-match path is ever reached. That is reasonable behavior (it fails during discovery, before torch is actually installed, rather than later during the real install), but the docs don't say so.
  Confidence 70 · mechanical · Fix: add one sentence noting that a resolve failure (as opposed to an unserved pin) fails the install rather than falling back.

Decisions for the author

• Resolving vLLM's full dependency graph once per install, purely to read two version lines — tradeoff
  resolve_vllm_dependencies always runs an extra, unconstrained (non---no-deps) uv dry-run resolve of the just-discovered vLLM pin, in addition to the existing per-package calls. This adds one network round-trip to every ROCm 10.x discovery install (even when nothing is broken), in exchange for pinning flash-attn/amd-aiter to versions vLLM's own metadata actually accepts. Given the failure this fixes previously broke every CI run in this lane, the added cost looks clearly worth it, but it's a real, permanent per-install cost worth the author confirming is acceptable going forward (e.g. if install latency is ever budgeted).

Positive signals

• discover_pinned_requirement_for was factored out of discover_pinned_requirement cleanly, and its error-detail construction was switched to the shared, already-tested rocm_core::command_failure_detail helper instead of re-deriving the same stderr/stdout-precedence logic inline a second time.
• The PR description is unusually explicit about what wasn't verified (local sudo/OpenMPI blocker stopping therock-next-09 short, Windows lane untested, which lane is the actual live guard) rather than glossing over the gaps.
• The new unit tests model the real historical failure (amd-aiter==0.1.24.post1 newest-in-series vs. the 0.1.22.post2.dev0 build vLLM 0.29 actually depends on) rather than synthetic placeholder versions, which documents the regression concretely for future readers.

Deployment notes

• E2E tests (MI300X, current) — the actual live regression guard for this exact bug (per both the PR description and the therock-next-09 scenario's own comments) — was still QUEUED as of this review, not yet completed. The WSL2 repro and the new unit tests are strong but do not exercise the real staging index; confirm this check goes green before merging. (The other self-hosted E2E lanes — Strix Halo Ubuntu release/nightly, W7900 — were also still queued/in-progress at review time; everything that had completed was green.)
• No migration, config, or public-API changes; this is purely an internal install-resolution behavior change inside engines/vllm.

What this covered

Read engines/vllm/src/install.rs and docs/vllm.md completely at PR head 61a18833 against merge-base efb9bc37672e9ca57966be7e30cd064061909348 (efb9bc37…61a18833), including the full surrounding (unchanged) code the new functions call into (discover_pinned_requirement, dry_run_resolved_pin, write_vllm_index_uv_config, ensure_rocm_local_version_matches, the three vllm_rocm10_discover_*_install_args builders) and the shared rocm-core helpers (uv_pip_install_base, command_failure_detail). Also read the relevant e2e coverage (tests/e2e-cucumber/features/therock_next_generation.feature's therock-next-09 scenario and its step definitions in therock_steps.rs), git log on the touched file for prior related fixes (#471, #583, #630), and the repo's AGENTS.md.

Ran one independent blind fan-out pass (a subagent reviewing the same diff cold, without this report's conclusions); its findings were verified against the source and folded into the list above.

Did not run: cargo test/clippy/fmt — no Rust toolchain or outbound network access available in this environment to install one; relied on reading the code directly, the PR's own reported local verification, and the PR's CI results (build-and-test, clippy, Test (affected crates), windows-build-and-test, Commit signatures + sign-off all SUCCESS at head 61a18833; self-hosted GPU E2E lanes still queued/in-progress, see Deployment notes). No ticket was named in the PR description, so there was nothing to retrieve from a tracker.

Not reconciled against the existing human review already posted on this PR (by juhovainio, CHANGES_REQUESTED, at the same head commit) — this report was produced independently and does not incorporate or respond to that review's content.

[volen-silo]

Thanks for the careful review. The branch is rebased onto current main and force-pushed (eb22702e). Point by point:

1. A failed probe aborted the install, but the docs said otherwise. You were right, and I changed the code to match the docs rather than the other way round. The probe only improves on the series match, so it should not make an install fail that worked before. A failed probe now falls back to the series pins for both companions, and the reason is reported in the install warnings. docs/vllm.md says so.

2. The confirm error was discarded. Fixed. A confirm call that fails now falls back to the series pin with a note that names the build the resolver wanted and the error. Because a note shown only on success is lost exactly when the series pins are what fail, the same notes are also added to the error if the real install then fails.

3. Tests and the scenario. Added three tests that drive a fake uv's resolve output through install_vllm_rocm10_discover (the 0.1.22 build reaches the real install; a failed probe falls back with its reason; an unconfirmed pin is reported and follows a failed install). I made a failed probe fatal again and the fallback tests failed, then restored the change. The PR text now names @id:therock-next-09-live-install-reports-vllm-rocm10x-discovery-pins as the covering scenario and says it is on the gated GPU lane. It fails on main on every MI300X run. Whether it passes with this change is not proven yet: this PR's MI300X run is still queued, and locally it stopped on the OpenMPI sudo prompt. The text says that.

4. "One uv call". Corrected. It now lists the probe, one confirm call per companion pin, and the series call used when a confirm misses.

5. docs/testing.md and docs/manual-testing.md. Left alone on purpose, and the PR text now says why: no command, flag, exit code or output format changes, only which pins are chosen and a warning when it falls back.

Thanks also for checking that uv pip install --dry-run --reinstall still prints the + pkg==ver line for an installed package.

Fixed.

[juhovainio]

No blocking issues from my read of the diff. The fix sits at the pin-selection layer, which is the right place, and the regression tests plus the cited e2e scenario cover it well.

Non-blocking notes, three of them inline:

• The e2e scenario that exercises the fix on the MI300X lane (therock-next-09) hasn't run yet and isn't in the check list. The fake-uv tests prove sequencing, not real uv behaviour against the staging index, so worth confirming that lane before merge.
• The probe resolves differently from the real install (--reinstall, no torch pins), so the "cannot succeed here and then fail there" wording is a bit strong.
• A transient confirm failure falls back to the known-bad series pair; one retry would help.
• The note ends up as the outermost error message; the tests use {:#} so they don't see it.
• The new tests are cfg(unix) only, so the Windows lane can't exercise these paths.

This reflects a diff read only, not a runtime check.

[juhovainio]

The probe resolves with --reinstall and without the torch pins, while the real install uses --reinstall-package vllm with torch already pinned. So the doc claim on resolve_vllm_dependencies that the probe "cannot succeed here and then fail there" is stronger than guaranteed. Only the companion lines are consumed, so the risk is low; consider softening the comment.

[juhovainio]

A transient failure of the confirm call drops to the series pin, which is the known-unsatisfiable pair on ROCm 10.1. It is reported in the note, but a single retry of the confirm call here would avoid the transient case.

[juhovainio]

err.context(notes...) makes the note the outermost message, so plain {} display leads with the note instead of the uv failure. The tests use {:#} and would not notice. Consider appending the notes after the cause, or asserting the plain display.

[nowycondro]

🔴 Automated review · pr-review-watcher · eb22702

Review — no blocking findings

Read engines/vllm/src/install.rs and docs/vllm.md completely at PR head eb22702e against merge-base efb9bc3767... (two independent passes: one on install.rs's correctness, one on the docs/history/comment-accuracy angle). The core idea is sound — pinning flash-attn/amd-aiter from the discovered vLLM's own dependency resolution fixes the stale-series-pin mismatch this PR exists to close, with a documented, tested fallback to the old series match on any probe/confirm failure.

Blocking

None

Non-blocking

• Fallback-reason notes are silently dropped on failure paths other than the one this PR special-cased — engines/vllm/src/install.rs (install_vllm_rocm10_discover, ~lines 1145-1201). Only the final run_uv_pip_install failure folds collected notes into the returned error via err.context(...). Every other ? that can fire after notes start accumulating (the second discover_vllm_companion_pin call, the per-pin ensure_rocm_local_version_matches loop, the torch install) discards whatever notes were already collected. No test exercises any of these combinations. Confidence 82 · logic. Fix: fold notes into every error return point in this function, not just the last one.
• The doc comment on install_vllm_rocm10_discover overstates note cardinality — it claims "a note for each companion pin that fell back to the series match" (1:1), but when the probe itself fails, both companions fall back while only one shared note is pushed — confirmed by the PR's own test a_failed_resolve_probe_falls_back_to_the_series_and_says_why asserting notes.len() == 1 for two fallen-back pins. Confidence 80 · mechanical.
• The orchestrator-level test for "confirm succeeds" uses a tautological fake uv — run_discover_with_fake_uv's confirm_ok=true branch echoes back whatever requirement string it's asked to confirm, for any input, so it can't distinguish "the vLLM index genuinely serves this exact build" from "the confirm step is broken and always claims success." A genuinely discriminating fake exists only at the lower-level helper (fake_uv_for_staging_index), which bypasses the real orchestrator. Confidence 80 · test-coverage.
• No orchestrator-level test covers a mixed outcome (one companion confirms, the other falls back) — the closest thing to the real-world bug this PR fixes, and the scenario most likely to interact with the dropped-notes issue above. Confidence 72 · test-coverage.

Decisions for the author

• The probe + confirm calls run unconditionally on every ROCm 10.x discover-route install/repair, adding a full dependency-graph uv resolve (network-bound) even when the known-bad pin combination isn't actually at risk for that install. Given the failure this fixes previously broke every CI run in this lane, the added cost looks worth it, but it's a permanent per-install cost worth confirming is acceptable going forward.

Positive signals

• discover_pinned_requirement_for was cleanly factored out and reuses the shared, already-tested rocm_core::command_failure_detail helper rather than re-deriving stderr/stdout precedence inline.
• The new unit tests model the real historical failure (amd-aiter==0.1.24.post1 vs. the actual 0.1.22.post2.dev0 build vLLM depends on), not synthetic placeholders.
• Learned from the repo's own history: PR #630 originally shipped an equivalent fallback note with no test, requiring a follow-up commit to retrofit one — this PR's equivalent notes are unit-tested from the first commit.
• docs/vllm.md's new paragraph correctly distinguishes "probe fails" from "confirm fails" (verified against the actual control flow — neither path silently falls back to an unpinned/PyPI build).
• The PR description is unusually explicit about what wasn't verified locally (sudo/OpenMPI blocker, Windows lane untested).

Deployment notes

CI is green across all required checks at this head (build-and-test, clippy, Test (affected crates), windows-build-and-test, Commit signatures + sign-off, Coverage, E2E tests/E2E consolidated report, docs build). The self-hosted MI300X/GPU-gated lane — the actual live regression guard for the bug this PR fixes, per the PR's own description — does not appear as a distinctly named check at this head; the fix is otherwise supported by unit tests and a WSL2 repro, not yet by a live run against the staging index. Worth confirming that lane before relying on this as closed. No migration/public-API changes; purely an internal install-resolution behavior change inside engines/vllm.

What this covered

Both changed files (engines/vllm/src/install.rs, docs/vllm.md) read completely at head eb22702e against the merge-base, including the surrounding unchanged code the new functions call into, via two independent passes (code-correctness; docs/history/AGENTS.md-adherence/comment-accuracy). Also read the relevant git log history (#471, #583, #630) and the e2e scenario this fix is meant to prove (therock-next-09). Did not run cargo test/clippy/fmt — no Rust toolchain or outbound package-registry access in this sandbox; relied on static tracing and the PR's own CI results. Not reconciled against the existing human review thread on this PR (by juhovainio, requesting changes at an earlier head) — this report was produced independently.

[volen-silo]

Thanks for the approval and the notes. I took the PR out of the merge queue to add these, so head is now c2bc2baa (rebased onto current main).

• The probe wording. Softened. The comment now says the probe uses the same default index and config file as the real install but not the same flags (--reinstall for the whole set, no torch pins), so it approximates the install and does not guarantee it, and that only the companion lines are consumed. The PR text says the same.
• Retrying the confirm call. Done: the confirm is tried twice before falling back. A new test uses a fake uv that fails the first confirm and succeeds the second, and expects the confirmed pin and no note. It fails with the retry removed.
• The note leading the error. Fixed: the notes are now appended after the install's own failure, so plain {} display leads with the uv error. The failed-install test checks the plain display as well as {:#}, and it fails with the old ordering.
• The tests being cfg(unix). Left as it is. The fake uv is a shell script, so the Windows lane would need a different fake, and the logic these tests cover is not platform-specific. The PR text says so.
• therock-next-09 not having run. Agreed, and it is still the only end-to-end proof: the fake-uv tests prove the call sequence, not uv against the staging index. This head's MI300X run is still queued, and I would wait for that lane to report on the scenario before merging.

docs/vllm.md is updated for the retry and the error ordering.

[nowycondro]

🔴 Automated review · pr-review-watcher · c2bc2ba

Review — needs work

Full review of the whole change (base fe1dbabe…head c2bc2ba, two files: engines/vllm/src/install.rs, docs/vllm.md). Implements the fix described in its own title (pin flash-attn/amd-aiter to what the discovered vLLM depends on); no ticket named.

Blocking

• New test helper reintroduces the exact ETXTBSY fork-race this file was already fixed for — engines/vllm/src/install.rs:2089-2106 (fake_uv_for_staging_index, used by companion_pins_follow_what_the_discovered_vllm_depends_on and companion_pin_the_vllm_index_does_not_serve_falls_back_to_the_series)
  Writes the fake uv script with raw std::fs::write(&path, script).unwrap() + std::fs::set_permissions(...), then the test executes it via production code. Commit 2314b2de ("test: write fake executables without the ETXTBSY fork race (#649)") removed precisely this pattern from this same file, citing "the vLLM ROCm 10 discover install tests" as an affected site, and replaced every write-then-run site with rocm_core::fake_executable::write_fake_executable — which every other new test in this diff correctly uses. Under default multi-threaded cargo test, this can intermittently fail with "Text file busy (os error 26)" when another test's child process holds the write fd open across the exec window, exactly the mechanism #649 documents.
  Confidence 85 · mechanical · Fix: replace the two raw std::fs::write/std::fs::set_permissions calls in fake_uv_for_staging_index with rocm_core::fake_executable::write_fake_executable(&path, script)?, matching every other fake-executable site in this file.

Non-blocking

• New probe/confirm dry-run calls omit --color never, inheriting a known silent-failure mode — engines/vllm/src/install.rs:674-720 (discover_pinned_requirement_for), :735-772 (resolve_vllm_dependencies)
  Commit a5e507a7 established in-code that an uncolorized exact-prefix match is required for uv pip freeze/uv pip check output parsing: under FORCE_COLOR/CLICOLOR_FORCE, ANSI codes break the exact-string match and the failure is silent. The new probe/confirm path parses uv's dry-run output with the same kind of exact-prefix matcher (dry_run_resolved_pin) and doesn't pass --color never either, so under that environment condition it would silently take the series-match fallback instead of the discovered pin. This is an inherited gap shared with the pre-existing sibling call sites through the same helper, not a new divergence this PR introduces, and it degrades gracefully (safe fallback, not a crash) rather than breaking the install.
  Confidence 70 · logic · Fix: pass --color never (or NO_COLOR=1) on the probe/confirm uv invocations, consistent with the existing uv_pip_freeze_args/uv_pip_check_args convention.
• Doc comment overstates note granularity on probe failure — engines/vllm/src/install.rs:1101 vs :1153-1163
  The doc comment on install_vllm_rocm10_discover says the function returns "a note for each companion pin that fell back to the series match." When the resolve probe itself fails, both flash-attn and amd-aiter fall back, but only one combined note is pushed (not one per pin) — the function's own test a_failed_resolve_probe_falls_back_to_the_series_and_says_why asserts notes.len() == 1 for this case. The claim only holds for confirm-level (per-package) fallbacks. Not user-facing or functionally wrong, just imprecise.
  Confidence 70 · mechanical · Fix: reword to "...and a note explaining any fallback to the series match (one combined note if the probe itself failed, one per package if a confirm did)."

Decisions for the author

• User-observable pin/warning behavior has no new Gherkin scenario, only Rust unit tests — answered-by-intent
  AGENTS.md §3: "User-observable behavior needs a scenario, not only a unit test... a unit test asserting the internal helper does NOT discharge this." No file under tests/e2e-cucumber/features/ is touched by this diff, and in isolation that would be blocking. But the PR description names the exact pre-existing scenario this exercises: "The scenario that covers it is @id:therock-next-09-live-install-reports-vllm-rocm10x-discovery-pins, tagged @requires-gpu @requires-engine:vllm... It fails on main on every run of that lane with the amd-aiter error above. Whether it passes with this change is what this PR's MI300X run will show; that run is still queued, so it is not yet proven." Verified: that scenario (asserting "the install reports the vLLM ROCm 10.x discovery pins", @requires-gpu) already exists at the base commit, unchanged by this diff — this is exactly the AGENTS.md-sanctioned path ("say in the PR text which @id: it is... if the scenario can only run on a gated lane, say so and name the lane"). Would otherwise be blocking. The live "E2E tests" check was still in progress at last check (gated GPU lane) — confirm it actually passes before merge; an honest "not yet proven" in the PR text is not the same as a green run.

Positive signals

• The retry-wrapped confirm (confirm().or_else(|_| confirm())) and the error-path note propagation are backed by a test that actually distinguishes "retried and succeeded" from "no retry" by state+message together (a_transient_confirm_failure_is_retried_once_before_falling_back), not a vacuous assertion — traced that deleting the retry branch changes both the returned pin and the note.
• Both failure points (probe failure, confirm failure) fall through to the exact same pre-existing discover_pinned_requirement(..., series) call used before this change, so the change is structurally incapable of making an install fail that would have succeeded under the old series-match-only logic.
• docs/vllm.md's new section was checked claim-by-claim against the code (probe reuses the real install's index/config file, confirmation tried twice, fallback reasons surfaced in both warnings and the final error) and matches precisely.

Deployment notes

• This only changes which flash-attn/amd-aiter build gets pinned during the ROCm 10.x vLLM discovery path (install_vllm_rocm10_discover); no flag, command, or output-format change. The new uv dry-run calls are confirmed non-mutating (--dry-run on both the probe and confirm paths).
• The gated MI300X E2E run this fix specifically targets (therock-next-09) was still queued/in-progress as of the last CI check here — the PR's own claim that main fails that scenario every run is the reason this fix exists, so merging ahead of that run returning green means the fix is unverified against the actual hardware failure it targets.

What this covered

Full review of both changed files at their complete current content (not just the diff hunks): engines/vllm/src/install.rs (2890 lines) read completely across two independent workers (production code — install_response, discover_pinned_requirement_for, resolve_vllm_dependencies, discover_vllm_companion_pin, install_vllm_rocm10_discover, is_amd_device_package_for, unverified_repair, install_vllm_with_uv — and the full mod tests block separately), plus docs/vllm.md in full. Diffed fe1dbabec03dd8ff5826f14f9c02dc0c3dde937f...c2bc2baa3b0a347f96004767aa5453db95339717. Four independent fan-out passes ran: production-code correctness, test-module correctness, AGENTS.md agent-instruction adherence, and history+code-comments (git log/blame on this file's prior commits 75127d7b, a5e507a7, 3067a103, 2314b2de, plus every comment in the touched regions). All findings above were re-verified directly against the file content by the consolidating pass before inclusion.

CI: 23 checks at last check, all green except windows-build-and-test and E2E tests still in progress; no failures observed. Impact analysis found no sibling install path (outside install_vllm_rocm10_discover) that pins flash-attn/amd-aiter and would need the same fix — the discover-build table is ROCm-10.x-specific and this is its only consumer. Did not run: a live cargo test -p rocm-engine-vllm reproduction of the ETXTBSY race (it's a confirmed-by-code-comparison finding, not a reproduced flake, since reproducing a fork-race intermittency reliably in this sandbox wasn't attempted). Not reconciled against any prior review discussion on this PR — this is a fresh blind pass per the current head, independent of any earlier round.