[rominf]

Summary

Lets an E2E scenario describe the GPU machine it needs (eight Instinct GPUs, a WSL2 distribution with GPU passthrough, a bare-metal host, a box with no GPU) instead of depending on the runner's hardware. Built on #510's ROCM_CLI_TEST_HOST_ROOT (now on main), scenarios that only need the CLI to see a GPU run on every Linux lane, including the GitHub-hosted E2E tests lane on every PR. Three commits:

1. E2E_HARDWARE=simulated|real and @requires-real-gpu.
   • @requires-real-gpu implies @requires-gpu and also skips unless the run sets E2E_HARDWARE=real.
   • The default is simulated. Any other value aborts the run, so a misspelt real can't quietly skip the real-GPU scenarios and still go green.
   • Every self-hosted GPU lane sets real: per-PR MI300X, Strix Ubuntu, Strix Windows, WSL and W7900; nightly also R9700 and MI350P. The WSL lanes forward it through WSLENV.
2. Simulated machines (tests/e2e-cucumber/src/simulated_host.rs).
   • A Given step plants a directory laid out the way the kernel lays out /dev, /sys and /proc: the KFD topology (gfx_target_version inside each node's properties), DRM cards, /dev/kfd//dev/dxg, /proc/version, /etc/os-release.
   • Commands get a PATH without the machine-describing tools (uname, lspci, ldconfig, rocminfo, amd-smi, mpirun, the kernel log, the WSL interop shells). The tools the simulated machine has come back as a fake-host-tool stand-in answering from canned output. The runner's GPU scoping (HIP_VISIBLE_DEVICES, ROCR_VISIBLE_DEVICES, ROCM_PATH, ...) is unset and HOME moves into the scenario.
   • Given a managed runtime is active on a simulated machine plants a runtime whose vllm is a fake-vllm stand-in, so a serve runs the CLI's real engine selection, runtime lookup, managed launch and readiness wait; only the process at the end is simulated.
   • Nineteen scenarios move onto simulated machines and @requires-os:linux: examine-04/10/11/17, diagnose-08/15/16/17/18, driver-install-01..03, serve-11/13/15/16/19/20/21.
   • Every scenario left on @requires-gpu needs a physical device or a runtime really installed for one, so those become @requires-real-gpu. examine-29 (@id:examine-detects-this-machines-gpu-and-driver) repeats examine-04 against the real machine on every GPU lane, the Windows one included.
3. A nightly drift check. examine-30 (bare-metal GPU host) and examine-31 (WSL2) hold every fixture assumption against the live /dev, /sys, /proc/version and lspci of a real machine, so a kernel or driver change that moves something fails there, naming the difference, instead of the simulated scenarios passing while the CLI fails on hardware.

Not simulated: package-manager state, discovered ROCm installs, and the identity of the user running the suite ($USER, id -Gn, stat on the planted device nodes). No simulated scenario asserts on them. Container detection does read under the host root, so a simulated machine is never in a container.

Risk: low for the product (no product code changes); medium for the suite, since it changes which lanes run 19 scenarios. Each moved scenario still asserts the same behaviour.

Scenario coverage (AGENTS.md §3)

No CLI behaviour changes. This PR changes which machines existing scenarios run against, and adds examine-29/30/31. The moved scenarios gain coverage: several used to run only on the WSL lane or multi-GPU hosts and now run on every Linux lane. The real-kernel cross-check is kept by examine-29 (every real GPU lane) and examine-30/31 (nightly).

Test plan

On this head, rebased on main at 1c33248:

• cargo xtask e2e, full default suite on a WSL2 machine with no GPU: 237 scenarios ran (72 resolved to skip: real GPU, nightly, lifecycle, WSL, Windows, docker), 2 expected xfails, 3 unexpected failures, each explained:

  • serve-11 failed because another process on that shared machine already listened on 127.0.0.1:11435. In a private network namespace it passes.
  • comfyui-04 (download progress frame) and automations-04 failed under load and pass when run alone.

• Every scenario this PR touches passed in that run except serve-11 (above). examine-29/30/31 need real hardware, so they skip there.

• cargo test -p e2e-cucumber --lib --bins --test feature_naming: 186 + 1 + 4 pass. cargo check -p e2e-cucumber --test e2e is clean.

• cargo test -p xtask: 317 pass.

• cargo clippy --workspace --all-targets and -p e2e-cucumber --test e2e with -D warnings, on Linux and on x86_64-pc-windows-gnu: clean. cargo fmt --check is clean.

• cargo test -p rocm / -p rocm-core were not run locally: this PR changes no product code. CI runs them.

• If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. (n/a; none of the moved scenarios has an xfail row)

• If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per docs/architecture.md. (n/a)

• Every new or changed user-facing message was read against the code path that runs after it. (n/a, no user-facing message changes)

[rominf]

Rebased onto main at 1c33248. Head is now 124b181. What changed:

• Dropped the first commit ("let e2e builds re-root the hardware probes"). test(core): let e2e builds re-root the hardware probes #510 landed it on main in a later form.
• E2E_HARDWARE: real is set on main's current lane set: per-PR MI300X, Strix Ubuntu, Strix Windows, WSL and W7900, plus nightly W7900. The per-PR R9700 and MI350P lanes no longer exist.
• Scenarios main added with @requires-gpu since this PR was written are now @requires-real-gpu: examine-19, serve-23, serve-24 and runtime-lifecycle-12. runtime-16 keeps main's @requires-os:windows, and therock-next-09 keeps main's removal of @nightly.
• diagnose-13 is no longer moved. vllm: point import failures at rocm diagnose #646 made its checker apply on WSL too, so it already runs on every Linux lane without a simulated machine. That makes 19 moved scenarios, not 20.
• This PR's examine scenarios are renumbered examine-29/30/31, because main took 18-28. Cross-references now use @id: instead of numbers.
• Removed this PR's own copy of the fixture-binary lookup in favour of main's fixture_binary::fixture_binary.
• Corrected docs that main's changes had made wrong:
  • Container detection now reads under the host root, so a simulated machine is never in a container.
  • The user's identity ($USER, id -Gn, stat on device nodes) is not simulated.
  • docs/ci-hardware-testing.md and the WSL lane comments no longer say the WSL diagnose and driver scenarios run only on the WSL lane.
• When the simulated vLLM never becomes ready, the serve-11 failure message now includes the service log.

Local results are in the updated description. The full mock suite ran 237 scenarios. The 3 unexpected failures were all local: a port already held on the shared test machine, and two load flakes that pass on their own. The PR stays in draft until required CI is green.

[tomastola]

Review — needs work

Full review of the whole change (base 1c33248c, head 124b1815).

Blocking

• examine-17's Then step can pass without asserting anything — tests/e2e-cucumber/tests/e2e/examine_steps.rs:1014-1022 (assert_gpus_match_kfd_nodes)
  The step returns silently when kfd_gpu_node_versions(...) is None or empty, and again when host_has_lspci is false. The scenario now plants eight simulated GPUs, so those early returns are exactly what hides a planted tree the CLI no longer reads. If the planting, world.host_path or the CLI's probe root regress, the scenario stays green. The sibling assert_gpu_target_matches_kfd got a "nothing may quietly decline" guard in this PR; this step did not.
  Confidence 70 · logic · Fix: when world.simulated_host is Some, assert the node count equals host.gpus.len() and fail instead of returning. Skip the lspci bail-out for simulated hosts.

Non-blocking

• fake-host-tool answers plausibly for argv nobody planted — tests/e2e-cucumber/src/bin/fake-host-tool.rs:43-49
  Lookup falls back from the full command line to the bare tool name, so uname -n or lspci with different flags return the canned -a / -nn -D text with exit 0. If the CLI changes its lspci flags, the stand-in will not disagree, which weakens the drift protection this PR is built around.
  Confidence 70 · logic · Fix: fall back to the bare name only for argument-less tools (mpirun, lsmod); otherwise exit 127 with a loud stderr message.
• Stale tag name in comments — .github/workflows/ci.yml:1527 says "@requires-gpu scenarios resolve to skip here", and features/dash.feature:329 says "@requires-wsl rather than @requires-gpu"; the skipping tag is now @requires-real-gpu.
  Confidence 85 · mechanical · Fix: rename in both.
• plant_simulated_host panics on a second call into the same root — simulated_host.rs:437 (link(), symlink already exists). Loud, but undocumented; two Given steps in one scenario would trigger it.
  Confidence 70 · mechanical · Fix: document it, or fail with a message naming the double plant.
• Unused Platform::Wsl2 { rocdxg_ready: false } variant — simulated_host.rs:137, 247-250, 317 has ldconfig/rocdxg branches nothing constructs, so they are untested dead code.
  Confidence 70 · mechanical · Fix: drop the variant and branches, or add a scenario that uses it.
• README does not tell GPU-box users about E2E_HARDWARE — the tag table mentions it, but the environment-variable table and run examples do not. A plain cargo xtask e2e on a GPU machine now skips every @requires-real-gpu scenario without saying why where people look.
  Confidence 70 · documentation · Fix: add an E2E_HARDWARE row (default simulated) and an E2E_HARDWARE=real cargo xtask e2e example.

Decisions for the author

• Coverage moves off real hardware, and Windows/macOS lose scenarios — tradeoff
  • serve-13 (serve-no-gpu-fails-fast) was @requires-no-gpu, so it ran on any no-GPU lane including Windows and macOS; it is now @requires-os:linux on a simulated host. examine-10 went from @requires-bare-metal (skips only on WSL) to @requires-os:linux, so --no-frameworks is no longer checked on Windows or macOS.
  • serve-13/15/16/19/20/21, examine-10/11/17, diagnose-08/15/16/17/18 and driver-install-01..03 no longer run against a real GPU or WSL host. Only examine-04 has a real twin (examine-29); examine-30/31 check layout assumptions only. Real masked-device serve refusals and real-WSL diagnose/driver-plan paths run in no lane.
  • The simulated run is hermetic and runs on every PR, which is a real gain. Cheap mitigations: a @requires-real-gpu copy of serve-15 and a @requires-wsl copy of diagnose-17, as examine-29 does for examine-04; keep a non-simulated no-GPU variant for Windows/macOS. Note the description's "each moved scenario still asserts the same behaviour" and "gain coverage" do not mention the Windows/macOS loss.
• Fake vLLM ignores everything except --host and --port — tradeoff (src/bin/fake-vllm.rs:30-36)
  serve-11 proves selection, runtime lookup, launch and readiness, but not that the CLI passes sane vLLM flags; --port=N would be ignored and fall back to 8000. Recording argv under the root and asserting the expected flags would close this; acceptable if the flags are covered elsewhere.
• serve-11 failed in the test plan on a port conflict (127.0.0.1:11435) — non-blocking-improvement
  A fixed port in a hermetic harness invites this on shared runners; an ephemeral port would remove the flake.

[nowycondro]

🤖 Automated review (Claude Code) · 124b181

I read the full diff for this PR (simulated-host infrastructure, expectation/tag plumbing, CI workflow changes, and the migrated e2e feature/step files), cloned the repo at the PR head commit, and cross-checked the two concerns from @tomastola's existing CHANGES_REQUESTED review against the current code. No Rust toolchain is available in this environment, so nothing was compiled or executed — everything below is from static reading of the diff and surrounding code.

Confirmed still open (both from the existing review)

• examine-17's assertion can still pass without checking anything — tests/e2e-cucumber/tests/e2e/examine_steps.rs:1012-1016 (assert_gpus_match_kfd_nodes). The function returns silently if kfd_gpu_node_versions(...) is None/empty, before ever comparing against the planted host. Compare this to its sibling assert_gpu_target_matches_kfd (same file, lines 945-956), which was given an explicit if let Some(expected) = world.simulated_host....gfx_target() { assert_eq!(...) } guard so "nothing below may quietly decline to check it." assert_gpus_match_kfd_nodes has no equivalent direct comparison against world.simulated_host.as_ref().map(|s| s.host.gpus.len()). The feature comment at features/examine.feature:226-227 ("the step's premises always hold") documents the intent but the code doesn't enforce it — a regression in planting, world.host_path, or the CLI's probe root would make this scenario pass vacuously instead of failing. Confidence: high (70-80%), this is a real gap, not a false positive.

• Coverage-loss tradeoff is real and still present — verified via git grep at head: features/model_serving.feature:135 (serve-13) now carries @requires-os:linux only, down from @requires-no-gpu (which also ran on Windows/macOS). features/examine.feature:111 (examine-10) is similarly now @requires-os:linux, down from @requires-bare-metal (which only skipped WSL). Both comments in the feature files candidly explain the simulated premise, but the net effect for those two scenarios is that Windows/macOS lanes no longer exercise GPU-required-refusal and --no-frameworks behavior. This is a tradeoff for the author/reviewers to accept or mitigate (e.g. a non-simulated Windows/macOS variant), not something I'd block on myself, but it is accurately described in the prior review and still applies at this head commit.

Additional observation (non-blocking, low confidence)

• tests/e2e-cucumber/src/simulated_host.rs (filter_real_path/link_stand_ins, around line 402) rebuilds the filtered-PATH fixture from scratch on every scenario invocation — a fresh read_dir+stat+symlink pass over every PATH directory per scenario. With 19+ scenarios now going through this path per run, this adds I/O overhead to the per-PR suite. This is a performance nit, not a correctness issue, and I did not measure actual wall-clock impact.

Not independently re-verified

I did not re-derive every item in the prior review's non-blocking list (fake-host-tool argv fallback, stale @requires-gpu tag mentions in comments, plant_simulated_host panic-on-double-call, unused Wsl2 { rocdxg_ready: false } variant, README gap, fake-vllm flag handling, serve-11 port conflict) — a spot read of the code is consistent with those being accurate, but I did not independently trace each one line-by-line given the medium-effort budget for this pass. I found no additional correctness bugs beyond what's above and what's already flagged.

🤖 Generated with Claude Code

[nowycondro]

🔴 Automated review · pr-review-watcher · 124b181

Review — needs work

Full review of the whole change, run as five independent file-grouped passes plus consolidation.
Implements ROCMAI-677 ("E2E: describe GPU hosts with simulated machines so GPU-visibility scenarios run on every PR"); the PR description's claims (HardwareMode gating, simulated_host.rs layout, fake-vllm/fake-host-tool stand-ins, nineteen scenarios moved to simulated machines, examine-29/30/31 real-hardware/drift checks) match what the diff contains, verified across passes with no mismatch found.

Blocking

• CI check "E2E tests (MI300X, current)" is failing on the reviewed commit — .github/workflows/e2e-selfhosted.yml (self-hosted MI300X lane); run: https://github.com/ROCm/rocm-cli/actions/runs/40314879688/job/121545019785
  Confirmed via gh pr view 517 --json statusCheckRollup (conclusion: FAILURE) and the rendered Actions page, which shows the "Run E2E tests on GPU hardware" step exiting with code 1; every other check on the PR is green. Full job logs were not retrievable from this environment (the GitHub Actions REST log/run API endpoints returned 403/Access Denied through an egress proxy), so the specific failing scenario or assertion could not be identified. This PR's own e2e-selfhosted.yml changes are what first set E2E_HARDWARE: real on this exact lane, so the failure may be the first real exercise of the new @requires-real-gpu gating on that hardware rather than an unrelated flake — that is plausible, not established.
  Confidence 100 · architectural · Fix: obtain the job's actual log/annotation (needs sign-in or maintainer access), determine whether it's a genuine regression in the new real-GPU gating/hardware-state assumptions or a known self-hosted-runner flake, and resolve or re-run before merge.

Non-blocking

• ldconfig cache header reports a fixed count of 5 libraries that doesn't match the number actually written — tests/e2e-cucumber/src/simulated_host.rs:308-320. The canned ldconfig -p output always prints "5 libs found in cache" but only 4 unconditional entries are written (a 5th, librocdxg.so, is added only for Platform::Wsl2 { rocdxg_ready: true }), so on bare-metal and WSL2-not-ready machines the printed header disagrees with the printed list below it. No consumer currently parses the header count (rocm-core's checks match by soname substring), so nothing currently breaks, but it's a fidelity gap in a fixture whose whole point is to match real ldconfig output.
  Confidence 80 · mechanical · Fix: compute the header count from the number of lines actually emitted rather than hardcoding it.
• bare_metal_layout_drift's GPU-topology checks early-return when the simulated side reports zero GPUs — tests/e2e-cucumber/src/simulated_host.rs:520-557 (let Some((_, simulated_props)) = simulated_gpus.first() else { return drift; }). Two drift checks (non-zero gfx_target_version on the CPU KFD node; DRM card vendor/device id shape) never execute when comparing against a no-GPU simulated host. Every current call site only drives this with instinct_mi300x(1), so it's unreachable today, but it's a latent gap if this drift check is ever reused for a no-GPU scenario.
  Confidence 70 · logic · Fix: run those two checks unconditionally, independent of whether the simulated side has a GPU.
• fake-host-tool.rs redeclares TOOL_OUTPUT_DIR and TEST_HOST_ROOT_ENV instead of importing them from simulated_host.rs — tests/e2e-cucumber/src/bin/fake-host-tool.rs:22,24 vs tests/e2e-cucumber/src/simulated_host.rs:38,41. The two copies are held in sync only by a "must match" doc comment with no compiler enforcement; the sibling binary fake-vllm.rs:17 proves a [[bin]] target in this crate can use e2e_cucumber::... to reach the crate's own lib, so this duplication isn't architecturally forced (unlike the separate, legitimately-forced duplication against rocm_core::hardware_root::TEST_HOST_ROOT_ENV, which fake-host-tool.rs cannot reach).
  Confidence 75 · mechanical · Fix: import the two constants from e2e_cucumber::simulated_host instead of redefining them.

Decisions for the author

• @id:examine-detects-gpu-and-driver (examine-04) keeps its scenario id while changing what it verifies — tradeoff. Before this PR the id denoted "verified against the real installed GPU" (rare, @requires-gpu-gated, self-hosted-only runs); after, the same id denotes "verified against a simulated Instinct GPU" (frequent, @requires-os:linux, every Linux PR lane), with a new, narrower id (examine-detects-this-machines-gpu-and-driver, examine-29) now covering what the old id used to mean, and a nightly drift check (examine-30/31) mitigating simulated/real divergence. This is well-reasoned and documented in the feature-file comments, but if any external dashboard or flaky-test tracker keys history by scenario id, examine-04's historical baseline will now be read against a very different population of runs. Worth confirming no such tracker exists, or that the discontinuity is acceptable.

Positive signals

• bare_metal_layout_drift, wsl_layout_drift, and lspci_drift (simulated_host.rs) are explicit, tested mechanisms for catching simulation/reality divergence — e.g. a regression test pins the exact gfx_target_version-in-properties vs standalone-file bug that motivated simulating hardware in the first place — directly addressing the core risk that a simulated-host suite could drift from what real hardware actually does.
• HardwareMode::parse (expectation.rs) fails closed: an unrecognized E2E_HARDWARE value panics the run rather than silently defaulting to simulated, so a typo can't quietly skip every real-GPU scenario while CI stays green — and the one call site (tests/e2e.rs) is wired to actually propagate that panic rather than swallow it.
• assert_vllm_default's new simulated-only readiness assertion (serving_steps.rs) closes a real gap: rocm serve exits 0 regardless of readiness, so without this check a simulated vLLM that never came up would have passed silently.
• feature_body (expectation.rs) reads the actual .feature file text to assert a scenario's premise line is really present, so deleting the underlying Gherkin step would fail this test rather than silently certifying stale coverage.

Deployment notes

None — no product code changes; this PR only changes which CI lanes exercise which existing e2e scenarios and adds new test infrastructure.

What this covered

Every file in the diff, read completely at PR head 124b1815e0ef5359513dece166627e3f81f9e04b against base d59f596f6e08297c207f008021eecb37defc3558 (32 files, +1991/-261), split across five independent file-grouped passes: (1) tests/e2e-cucumber/src/simulated_host.rs; (2) tests/e2e-cucumber/src/expectation.rs + mock_server.rs + lib.rs; (3) tests/e2e-cucumber/src/bin/fake-host-tool.rs + fake-vllm.rs + tests/e2e/simulated_host_steps.rs + tests/e2e.rs; (4) the remaining tests/e2e/*_steps.rs files plus all ten changed .feature files; (5) docs/testing.md, docs/ci-hardware-testing.md, the three .github/workflows/*.yml files, tests/e2e-cucumber/Cargo.toml, tests/e2e-cucumber/README.md. Each pass also read relevant surrounding/consumer code outside its primary scope (e.g. crates/rocm-core/src/examine.rs, openmpi.rs, apps/rocm/src/service_launch.rs, engines/vllm/src/runtime.rs) to verify specific cross-file claims. Step 7 (design review) questions were answered from material the five passes already surfaced (scenario-id continuity, simulation fidelity, real-vs-simulated gating design) rather than a separate fan-out round.

CI: gh pr view 517 --json statusCheckRollup read in full; one check (E2E tests, MI300X, current) reports FAILURE, all others SUCCESS/SKIPPED — reflected above as a blocking finding. Job logs were not retrievable (GitHub Actions REST log/run endpoints returned 403 from this environment's egress proxy); only the rendered Actions page's step name and exit code were available.

Did not run: the history pass (git log/git blame on modified regions) and prior-changes pass (review comments on earlier changes to these files) were not run this round — simulated_host.rs, the fake binaries, and the simulated-host step definitions are new files with no history, and the remaining changed files' history was not separately mined given the depth already reached by the code and impact-analysis passes; re-run these specifically if deeper confidence is wanted. No cargo build/cargo test/clippy was executed in this environment; all findings were settled by reading source against source, not execution — the PR's own description reports cargo test, clippy, and a 237-scenario local run as green except three explained, non-blocking flakes (serve-11, comfyui-04, automations-04), consistent with what the diff shows but not independently re-run here.

Ticket ROCMAI-677 retrieved by key (title, status, issue type only — no comments): "E2E: describe GPU hosts with simulated machines so GPU-visibility scenarios run on every PR," status In Progress. Matches the change's own description of itself.

Not reconciled against any PR discussion, review comments, or earlier review rounds — none were read, per method.

[rominf]

@tomastola thanks. Rebased onto main and addressed in 3cd2e06, with the scenario numbering and tags in 6a09714:

• examine-17 could pass vacuously: its step now fails on a simulated machine unless it reads back as many KFD GPU nodes as were planted (the lspci bail-out was already skipped there). Pointing the probe at a missing directory turns it red; the old code passed silently.
• fake-host-tool's bare-name fallback: it answers only the exact command line it was planted for, with powershell.exe the one argv-independent exception, and exits 127 naming anything else. a_stand_in_answers_only_the_command_line_it_was_planted_for goes red if the fallback is widened again.
• Stale @requires-gpu comments in ci.yml and dash.feature now say @requires-real-gpu, and main's seven newer scenarios that need a physical device are tagged @requires-real-gpu too, so no feature file carries plain @requires-gpu.
• A second simulated-host plant fails fast, naming both machines.
• The unused Wsl2 { rocdxg_ready: false } variant is gone.
• README: an E2E_HARDWARE row and an E2E_HARDWARE=real cargo xtask e2e example.

From the later automated review: the ldconfig header count is computed from its entries, the drift check's CPU-node/DRM checks run without a simulated GPU, and fake-host-tool imports its constants. After the rebase the PR's scenarios are examine-31/32/33 (main now has examine-29/30). The MI300X failure was therock-next-09's vllm/amd-aiter pin mismatch, fixed on main by #755 and picked up by the rebase.

The coverage trade-offs you raised (twins for serve-13/serve-15/diagnose-17, fake-vllm flags, serve-11's fixed port) I'll answer separately.

[nowycondro]

🤖 Automated review (Claude Code) · 6a09714

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — needs work

Full review of the whole change (base 9875940e, head 6a09714). Implements ROCMAI-677 ("E2E: describe GPU hosts with simulated machines so GPU-visibility scenarios run on every PR"); the ticket title/status match the PR's own description, and the description's claims (HardwareMode gating, simulated_host.rs layout, fake-vllm/fake-host-tool stand-ins, nineteen scenarios moved to simulated machines, examine-29/32/33 real-hardware/drift checks) match what the diff contains.

Earlier rounds on this PR (at older commits) flagged a vacuous-assertion gap in assert_gpus_match_kfd_nodes and an MI300X CI failure. Both are resolved at this head: assert_gpus_match_kfd_nodes now carries the same "never decline to check on a simulated host" guard its sibling assert_gpu_target_matches_kfd already had, independently re-verified by two blind passes below. CI has moved on to a different, newly-identified failure (below).

Blocking

• A #[given(...)] step-text rename broke 3 Windows-only scenarios, and this is the confirmed root cause of the live CI failure on the Windows self-hosted lane — tests/e2e-cucumber/tests/e2e/examine_steps.rs:67 renamed the step from "a machine with an AMD GPU" to "this machine has an AMD GPU" (confirmed against the base commit, which still has the old text). Two of the Gherkin call sites were updated to match (examine.feature:486, runtime_setup.feature:186), but three were not: diagnose.feature:647 (@id:diagnose-windows-display-driver-advice-follows-the-inspection, scenario diagnose-41), diagnose.feature:755 (@id:diagnose-windows-hip-sdk-advice-follows-the-inspection, scenario diagnose-48), and examine.feature:453 (@id:examine-windows-driver-version-date-and-age, scenario examine-29) still say Given a machine with an AMD GPU, which no longer binds to any step function. All three are tagged @requires-os:windows @requires-real-gpu, which is exactly why this was invisible in the author's local WSL2 test run (those scenarios skip there) and only surfaces on the Windows GPU lane.
  Verified by direct evidence, not just reading: the "E2E tests (Strix Halo, Windows, current)" job on this exact commit (run 41652611022, job 125974649969) is FAILURE, and its log's own reconciliation output names precisely these three scenarios — 'diagnose-windows-display-driver-advice-follows-the-inspection' was expected to pass on this host but FAILED — a regression., 'diagnose-windows-hip-sdk-advice-follows-the-inspection' was ... FAILED, 'examine-windows-driver-version-date-and-age' was ... FAILED — a 1:1 match with the three stale call sites.
  Confidence 100 · mechanical · Fix: update the three Given lines to Given this machine has an AMD GPU.

• CI is not green on this head beyond the above, and the gaps need triage, not just the rename fix — the same Windows job also reports a 4th failure, 'runtime-resolve-canonical-release' (runtime_setup.feature:186, tag @requires-gfx-target), which already uses the correct renamed step text, so it is not explained by the finding above; its cause is still open. Separately, two other self-hosted jobs — "E2E tests (Strix Halo, Ubuntu, release, current)" and "E2E tests (Strix Halo, Ubuntu, nightly, current)" — are also FAILURE, but their logs show ##[error]No space left on device during action checkout, i.e. runner disk exhaustion unrelated to this PR's code. Two more self-hosted jobs ("E2E tests (MI300X, current)", "E2E tests (W7900, current)") are still QUEUED and have not reported at all as of this review. So: one confirmed code regression (above), one unexplained scenario failure, two infra flakes needing a runner-side fix or re-run, and two jobs pending — the PR cannot be called green yet on any of these grounds independently.
  Confidence 100 (all read directly from the live check rollup and job logs) · architectural · Fix: fix the rename (above); investigate runtime-resolve-canonical-release separately; re-run or escalate the two disk-space failures; wait for MI300X/W7900 to complete before merge.

Non-blocking

• SimulatedGpu::gfx_target() formats the packed version's "minor" digit in decimal, not hex, diverging from the production formula it exists to mirror — tests/e2e-cucumber/src/simulated_host.rs:138-142 (format!("gfx{}{}{:x}", v / 10_000, (v / 100) % 100, v % 100)). The production decoder it stands in for, gfx_target_from_gc_version (crates/rocm-core/src/host_gpu.rs:1568-1573), hex-formats both the minor and revision digits (format!("gfx{major}{minor:x}{revision:x}")) — its own test (gc_version_encodes_components_above_nine_as_hex_digits) exists specifically because decimal and hex only agree below 10. Verified directly by comparing both formulas side by side. Currently dormant: the module's only two GPU constants (mi300x: minor=4, strix_halo: minor=5) have single-digit minors, so decimal and hex coincide and no test catches it — but this is test-oracle logic consumed by examine_steps.rs and serving_steps.rs to compute the expected value the CLI's real output is checked against, so a future GPU constant with minor ≥ 10 would silently assert the wrong expectation.
  Confidence 85 · logic · Fix: change to format!("gfx{}{:x}{:x}", v / 10_000, (v / 100) % 100, v % 100), matching gfx_target_from_gc_version.
• TEST_HOST_ROOT_ENV is duplicated between simulated_host.rs and rocm_core::hardware_root with only a "must match" doc comment, not a compiler/test-enforced invariant — tests/e2e-cucumber/src/simulated_host.rs:40-42. A working precedent for closing exactly this gap already exists in the same crate: tests/e2e-cucumber/src/mock_server.rs's own #[cfg(test)] block already reaches the rocm-core dev-dependency (rocm_core::ManagedServiceRecord), so a #[test] asserting TEST_HOST_ROOT_ENV == rocm_core::hardware_root::TEST_HOST_ROOT_ENV is reachable the same way. If the two literals are ever edited independently, the CLI binary would silently stop reading the simulated-host directory this whole PR plants, with nothing to notice.
  Confidence 75 · architectural · Fix: add that one-line equality test to simulated_host.rs's existing mod tests.
• bare_metal_layout_drift's reference-node selection relies on non-deterministic read_dir order — tests/e2e-cucumber/src/simulated_host.rs:530-552 (simulated_gpus.first(), no sort). The sibling production function this mirrors (detect_kfd_gfx_target_in in crates/rocm-core/src/host_gpu.rs) explicitly sorts nodes before picking "first," with a comment noting read_dir order is filesystem-defined. This drift-check helper has no equivalent sort, so its choice of "the" simulated GPU node to compare every real node's properties against could vary run to run once a simulated host has ≥2 heterogeneous GPU nodes. Dormant today: every current caller uses either a single-GPU host or homogeneous multi-GPU hosts.
  Confidence 70 · mechanical · Fix: sort gpu_nodes's entries (e.g. by node directory name) before taking .first().
• SimulatedHost's public platform/gpus fields let a caller construct a self-inconsistent machine — tests/e2e-cucumber/src/simulated_host.rs:166-171. Nothing stops SimulatedHost { platform: Platform::Wsl2, gpus: vec![SimulatedGpu::mi300x(..)] } outside the three documented constructors; plant_wsl2 (same file, 379-393) ignores self.gpus entirely and unconditionally writes the Strix-Halo-specific Windows-inventory line, so such a host's reported GPU and its simulated Windows inventory answer would contradict each other. Not currently reachable — every call site goes through instinct_mi300x/bare_metal_without_gpu/wsl2_strix_halo.
  Confidence 70 · architectural · Fix: make the fields private behind the existing constructors, or have plant_wsl2 derive its answer from self.gpus.
• ScenarioDecl.requires_real_gpu ⇒ requires_gpu is enforced only inside from_tags, not by the type — tests/e2e-cucumber/src/expectation.rs:111-219 (public independent bool fields), set together only inside the tag-parsing loop (:259-263); resolve()'s two gates key off requires_gpu alone and never re-derive it from requires_real_gpu. The file already has precedent for partial struct-literal construction of a ScenarioDecl elsewhere in its own tests, so a future hand-built literal could silently desync the two flags, which would make a @requires-real-gpu scenario resolve to ExpectPass on a GPU-less real-hardware host instead of skipping. Not currently reachable — the only production constructor is from_tags.
  Confidence 70 · architectural · Fix: compute requires_gpu from requires_real_gpu (private field + accessor), or debug_assert! the implication at the top of resolve().

Decisions for the author

• Coverage moves off real/WSL hardware for most of the 19 migrated scenarios, narrowing two of them from a broader platform tag to Linux-only — tradeoff. Verified directly against the diff: serve-13 (serve-no-gpu-fails-fast) moved from @requires-no-gpu to @requires-os:linux; examine-10 (examine-can-skip-framework-probing) moved from @requires-bare-metal (which only skipped WSL) to @requires-os:linux. Of the nineteen moved scenarios, only examine-04 keeps a real-hardware twin (examine-29); the nightly checks (examine-32/examine-33) verify fixture-layout fidelity, not scenario behavior, on real hardware. This is a reasonable tradeoff — hermetic, always-on coverage gained vs. real/WSL exercising lost for most moved scenarios — and the PR description's framing ("the moved scenarios gain coverage") doesn't mention the Linux-only narrowing for these two. Worth the author confirming this is accepted as-is rather than mitigated with a @requires-real-gpu/non-simulated twin for either.
• The simulated vLLM stand-in ignores every flag except --host/--port — tradeoff (tests/e2e-cucumber/src/bin/fake-vllm.rs). serve-11 genuinely proves engine selection, runtime lookup, managed launch, and readiness-wait (the fake binds the CLI-passed host/port rather than a hardcoded default, confirmed against engines/vllm/src/process.rs's vllm_serve_args), but does not prove the CLI passes correct additional vLLM flags — a wrong --max-model-len or similar would go unnoticed here. Acceptable if flag correctness is covered elsewhere; worth confirming that scope boundary is intentional.

Positive signals

• assert_gpu_target_matches_kfd and assert_gpus_match_kfd_nodes (examine_steps.rs) both now correctly guard every early return against vacuously passing on a simulated host — independently re-traced by two blind passes in this round, each confirming no early-return path discharges examine-04/examine-17 without having actually checked the planted topology.
• fake-host-tool's dispatch answers only the exact command line it was told to expect, with one narrow, explicitly-justified exception (powershell.exe, which must accept a whole inventory script) — directly defeating the "stand-in silently papers over a changed CLI invocation" failure mode, backed by a dedicated unit test (a_stand_in_answers_only_the_command_line_it_was_planted_for).
• bare_metal_layout_drift/wsl_layout_drift/lspci_drift, exercised nightly against real hardware (examine-32/33), is a well-targeted regression guard: its own test suite pins the exact historical bug (gfx_target_version read from a standalone file no kernel has) that motivated building this simulated-host infrastructure in the first place.
• HardwareMode::parse fails closed on an unrecognized E2E_HARDWARE value (panics rather than defaulting to simulated), consistent with this project's no-silent-GPU-fallback convention, and the one production call site (tests/e2e.rs) propagates that panic rather than swallowing it.
• tui_driver.rs's isolate_env_removals() addition closes a real, pre-existing isolation gap (a PTY-driven serve on a simulated host could previously inherit the runner's own HIP_VISIBLE_DEVICES/ROCR_VISIBLE_DEVICES), bringing the PTY path in line with the piped path's existing isolate_cmd guard.
• Docs/CI consistency is well maintained: docs/testing.md, docs/ci-hardware-testing.md, and tests/e2e-cucumber/README.md all describe the current @requires-real-gpu/E2E_HARDWARE semantics without drifting from each other or from the actual tag names in the source, and every self-hosted GPU job in both workflow files correctly sets/forwards E2E_HARDWARE: real.

Deployment notes

No product code changes — this PR only changes test infrastructure and which CI lanes exercise which existing e2e scenarios. Do not merge until the Blocking items are resolved: the confirmed step-text regression, the unexplained runtime-resolve-canonical-release failure, the two disk-space infra failures, and the two still-pending self-hosted jobs (MI300X, W7900).

What this covered

Every file in the diff (33 files, +2150/-282) read completely at PR head 6a0971404e3ddd4738074745f26c841d3f1613ed against merge-base 9875940ebc7d1dbf9db0706e01d29842e8ec1d27, by six independent blind fan-out workers grouped by coupling: (1) simulated_host.rs; (2) expectation.rs + mock_server.rs + lib.rs; (3) the fake binaries + simulated_host_steps.rs + tests/e2e.rs; (4) diagnose/driver/examine step files + their feature files; (5) serve/runtime/comfyui/tui step files + their feature files; (6) docs + CI workflow files + Cargo.toml + README. Each worker also read relevant surrounding/consumer production code (crates/rocm-core/src/{host_gpu,examine,diagnose,fix}.rs, apps/rocm/src/service_launch.rs, engines/vllm/src/{runtime,process}.rs) to verify cross-file claims rather than trusting the test code's own framing. I then independently re-verified the highest-value candidates myself (the step-text rename, the gfx_target hex/decimal divergence, the serve-13/examine-10 tag narrowing) directly against the source and the live CI logs before scoring them.

CI was read live via gh pr view 517 --json statusCheckRollup and GitHub Actions job logs, twice (once mid-review, once immediately before publishing) — the head commit had not moved between the two checks.

Caveat on blindness: before delegating the file-grouped passes above, this orchestrating context necessarily read the full bodies of three prior reviews on this PR (pinned to an older commit, 124b181) in order to perform the mandated check for whether this PR had already been reviewed at its current head — it had not, since the head has since advanced to 6a09714. That prior-review content was used only to decide whether to proceed, and afterward to sanity-check which previously-raised issues are now fixed (confirmed: the assert_gpus_match_kfd_nodes vacuous-pass gap and the ldconfig header-count mismatch are both resolved at this head); it was not given to any of the six fan-out workers, who read the code fresh with no knowledge of prior rounds, and none of this review's findings reproduce a prior round's finding. Did not run: the history/prior-changes passes were not run as a separate dedicated pass beyond git log spot-checks already folded into the workers' own investigation, since most touched files are new. No cargo build/cargo test/clippy was executed in this environment (no Rust toolchain available) — all non-CI findings were settled by reading source against source, not execution. Not reconciled against any PR discussion or review comments — none were read as part of producing these findings, per method.

---

🤖 Generated with Claude Code