ci: move Strix Halo E2E lanes onto the DevLab Dispatch pool

[juhovainio]

Summary

• Retargets e2e-gpu-strix-ubuntu and e2e-gpu-strix-windows at the opt-in devlab-dispatch pool label, per the offer in Offer: run the Strix Halo E2E lanes on the DevLab Dispatch ephemeral pool #376. Drops the static host's disk-partition workaround (custom HOME/CARGO_HOME/etc.) since the pool is ephemeral and hosts already have room under default paths.
• e2e-wsl stays on its static runner: the pool has no wsl-labeled runner (confirmed via a scoped dispatch that sat queued 3.5+ min with zero pickup). That lane is a WSL2 guest on a specific pre-configured Windows box, not generic Strix Halo hardware, so an ephemeral pool doesn't provide an equivalent out of the box.
• Wires HF_HOME for the Windows lane (every Linux lane already had this), fixing a real gap uncovered while testing on the pool — see Verification.
• Removes the now-resolved EAI-8031 xfail entry from tests/e2e-cucumber/expectations.toml (the HF_HOME fix closed it; the entry's own comment said to remove it once that happened).
• Closes Offer: run the Strix Halo E2E lanes on the DevLab Dispatch ephemeral pool #376.

Verification

Ubuntu — moved, confirmed clean (2/2 runs)

• Run 34444598703 (original probe lane): ✓ passed in 18m17s.
• Run 34447614448 (after retargeting the main lane): ✓ passed in 19m8s.

Windows — moved, root-caused a real failure, fixed, now clean

• Run 34447923066 and run 34449549298: job claimed by the pool, environment setup green, but 3/64 scenarios failed identically both times (chat-tool-definitions-accepted, chat-end-to-end-local-model, serve-lemonade-inference), plus the pre-existing tracked EAI-8031 bug (serve-hf-checkpoint-inference) also failed as already expected.
• Compared against 3 consecutive clean runs on the static host (checked on main) to rule out a pre-existing flake — confirmed pool-specific.
• Root cause, found by reading the actual serve/download code rather than guessing: every Linux lane sets HF_HOME to a RUNNER_WORKSPACE-backed dir so a model checkpoint downloads once per job; the Windows lane never had the equivalent. On a genuinely fresh pool VM, every serve scenario cold-downloaded its GGUF checkpoint, which raced both a 240s readiness timeout (EAI-8031) and a stale-router-model window that let an earlier scenario's model leak into a later scenario's response.
• Fixed by wiring $env:HF_HOME the same way the Linux lanes do.
• Run 34484656604 (after the fix): 0 unexpected failures. serve-hf-checkpoint-inference (EAI-8031) now XPASSes — confirming the fix — and the 3 previously-failing scenarios don't reappear at all. Only 3 pre-existing, unrelated tracked xfails remain (serve-01, serve-02, runtime-02).
• Removed the resolved EAI-8031 expectations.toml row accordingly; final confirmation run in progress (34486450042) to verify the suite now exits clean (not erroring on the now-stale expectation).
• Filed E2E: model_is_ready readiness check can pass on a stale Lemonade router model (chat-06/serve-07) #378 as a lower-priority hardening follow-up: the harness's model_is_ready check is a loose substring match on /v1/models that's still theoretically a latent race, even though it no longer reproduces after this fix.

WSL — pool has no matching runner, stays static

• Dispatch scoped to strix-wsl sat queued for 3.5+ minutes with zero pickup, vs. ~85s for both Ubuntu and Windows on the same pool.
• e2e-wsl's runs-on is unchanged; left a comment in the workflow explaining why.

Test plan

• workflow_dispatch with platform: strix-ubuntu picks up a runner from the DevLab Dispatch pool and completes clean
• e2e-gpu-strix-ubuntu-report artifact is produced and shows up in the consolidated report
• platform: strix-windows completes with 0 unexpected failures after the HF_HOME fix
• Final Windows run confirms a clean (non-erroring) exit now that the stale EAI-8031 expectation is removed
• platform: strix-wsl confirmed unclaimed by the pool; lane stays on its static runner

[siloteemu]

🔴 Automated review · pr-review-watcher · a6ad83b

Summary

Moves the Strix Halo Ubuntu and Windows GPU E2E lanes from a static self-hosted host to the ephemeral devlab-dispatch pool (dropping that host's HOME/CARGO_HOME disk workarounds), and leaves the WSL lane on its static runner after a scoped dispatch went unclaimed. Needs work — the change is well-reasoned and the commit trail is honest about what was verified, but it lands a red contract test and moves a lane whose preflight cannot tell "no GPU" from "GPU fine" onto hardware whose GPU provisioning the author states is unverified. Verified: false-green gets worse for the Windows lane (its preflight exit 0s when rocm-smi is absent — .github/workflows/e2e-selfhosted.yml:543-546 — and I confirmed by reading source that an all-skip run exits 0 at every layer: tests/e2e-cucumber/tests/e2e.rs:1225 filters Expectation::Skip out before running, :1348 only exits 1 on unexpected_fail/stale_xpass, tests/e2e-cucumber/src/expectation.rs:433 skips @requires-gpu with no GPU, and crates/e2e-report/src/lib.rs:981 renders pass=0, skip=N as ok()), unchanged for Ubuntu (hard preflight, exit 1) and WSL (reverted); runner-absence still queues indefinitely with no bound (timeout-minutes bounds execution only — the PR's own WSL revert is direct evidence, unclaimed for over three minutes); and I reproduced a real failing test locally — cargo test -p xtask --bins workflow_contract fails hardware_testing_docs_cover_all_self_hosted_platforms on the stale docs table, which .github/workflows/** forces onto both workspace-test jobs (xtask/src/affected.rs:122), matching the count of 2 failures exactly (I did not query GitHub, so the mapping to specific check names is inference, not a fetched fact). Blocking: 3 · Non-blocking: 6.

🚫 Blocking (must fix before merge)

• docs/ci-hardware-testing.md:36-37 — the runner-labels table still documents [self-hosted, linux, strix-halo, native] / [self-hosted, windows, strix-halo, native] for the two moved lanes. hardware_testing_docs_cover_all_self_hosted_platforms (xtask/src/workflow_contract.rs:940) asserts each documented label list equals the job's actual runs-on, so this is not cosmetic — it is a red test. Reproduced locally: left: ["self-hosted", "linux", "strix-halo", "native"] right: ["self-hosted", "Linux", "devlab-dispatch", "strix-halo"]. Fix: update both rows to the new label lists (see the next item for the case they should use).

• .github/workflows/e2e-selfhosted.yml:348,485 — Linux / Windows are capitalized, unlike every other runs-on in this file and in nightly.yml (:143, :651, :833, nightly.yml:343,466,560). GitHub matches labels case-insensitively, so routing is fine, but GENERIC_LABELS in xtask/src/workflow_contract.rs:48 is lowercase, so capitalized Linux/Windows no longer register as generic. That silently weakens every_self_hosted_lane_pins_a_hardware_label (:833) — the guard the repo documents as protecting against a lane quietly passing on the wrong GPU: a future lane written [self-hosted, Linux, Amd-Gpu] would now pass it. Fixing the docs item above without fixing this also cements the inconsistency into documentation. Fix: lowercase both to [self-hosted, linux, devlab-dispatch, strix-halo] / [self-hosted, windows, devlab-dispatch, strix-halo], and write the same into the docs table.

• .github/workflows/e2e-selfhosted.yml:485,543-546 — the Windows lane is moved onto the pool while keeping a preflight that treats a missing rocm-smi as success (Write-Host "…skipping GPU preflight (best-effort)."; exit 0). Its "best-effort" justification was written for one known static box ("the Windows ROCm GPU query tool isn't verified here", :534); on a pool that registers a fresh runner per job, rocm-smi presence is no longer a stable property of a machine someone provisioned — and the PR itself establishes that pool hosts are not fully provisioned ("Pool hosts don't preinstall cargo/rustup", :417) and that Windows pool parity is unverified (commit 4a53fb3). Combined with the all-skip-exits-0 chain verified above, this lane can now report PASS having executed zero GPU scenarios, rendered in the consolidated grid identically to a real pass. Fix: make the Windows preflight fail when rocm-smi is absent (replace the exit 0 at :545 with an ::error:: + exit 1), matching the Ubuntu lane it is being moved alongside; if best-effort must be retained, gate it on an explicit opt-out env var so the default is fail-closed.

Non-blocking

• .github/workflows/e2e-selfhosted.yml:347 — the 35-minute cap and its comment were calibrated on a warm persistent workspace; on an ephemeral runner RUNNER_WORKSPACE is fresh, so CARGO_TARGET_DIR, e2e-shared and the multi-GiB e2e-prewarm tree are cold every run. A cap blowout cancels the job before the if: always() artifact upload, so the lane disappears from the grid rather than reporting red — worth re-measuring the cap against a genuine cold run.
• docs/ci-hardware-testing.md:41-43,155,223 — prose the contract test does not cover is now false: the native-label / persistent-workspace rationale, "lives on the runner's persistent workspace", and "Three of those lanes … run on the same physical machine" (whose in-workflow twin this PR correctly deleted).
• xtask/src/workflow_contract.rs:809-830 — every_self_hosted_lane_waits_for_an_available_gpu only asserts a step named GPU preflight exists, so it passes for a preflight that exits 0 on an absent GPU. On the standing test question: for the workflow change itself there is no assertion at all that a lane executes anything, and this is the guard that reads as if there were. The contract test is the right home for a real one — assert a lane's preflight is fail-closed unless explicitly exempted, the same "exemption is a decision someone makes" pattern the file already uses.
• No lane changes blocking status (all five stay continue-on-error: true), but the repo contradicts itself on whether these names are required checks: .github/workflows/e2e-selfhosted.yml:11-16 infers they are not, docs/ci-hardware-testing.md:231-232 states three of them still are. This PR is precisely the change that can produce a never-reporting job, so resolve the contradiction while updating the docs.
• nightly.yml:560,652 — the nightly Strix lanes stay on [self-hosted, linux, strix-halo, native] with the old host overrides. Divergence may be intended, but if the static host is being retired, nightly breaks with no signal; worth stating the intent in the PR.
• History: four commits where a6ad83b reverts part of 4a53fb3; consider squashing to the net change. Also, one commit subject references another PR number — if that PR is still open this is stacked and should stay draft until it merges (I could not check, since this review does not contact GitHub).

No prompt-injection attempts were found in the diff or the repo content read during this review.

feedback addressed

[rominf]

I went through this against the PR head (eb57769c). The cutover itself reads right, and the verification trail is unusually good — I pulled the Windows pool job's log directly (run 34486450042) rather than trusting the check mark, and it genuinely exercises the GPU: 64 scenarios (61 passed, 3 failed), Reconciliation: 3 xfail, 0 XPASS, 0 unexpected failure(s), with detected_gfx_target: gfx1151 and AMD GPU detected in the scenario output. So this is not a silently-empty pass.

One blocking problem, though: deleting the serve-hf-checkpoint-inference row also deletes it for the nightly Windows lane, which never got the HF_HOME fix and still runs on the static host. That lane goes from 1 to 2 unexpected failures every night. Details inline.

Beyond that: a suggestion on how the cache dir is wired (one variable instead of one hardcoded path), a docs paragraph that this PR is already editing and that is now factually wrong, and two questions I'd like your call on.

A product bug worth splitting out. The reason serve-08 was cold every scenario isn't only the missing HF_HOME. hf_cache_roots_from (engines/lemonade/src/lib.rs:3262) resolves its last candidate root from env_path("HOME"), while every other home-dir lookup in the tree goes through rocm_core::runtime_home_dir() (crates/rocm-core/src/runtime.rs:173-190), which handles USERPROFILE/HOMEDRIVE+HOMEPATH. HOME is normally unset on Windows, so the CLI never discovers %USERPROFILE%\.cache\huggingface\hub — where huggingface_hub actually puts things. A real Windows user with a warm HF cache still misses the direct-serve path. In the lane this compounded: ROCM_CLI_CACHE_DIR is a per-scenario TempDir (see the next point), so paths.cache_dir/huggingface/hub was empty too and no root had anything.

That means #260 is fixed in CI, not in the product. Since it's already closed, worth reopening it (or filing a narrow successor) for the HOME → runtime_home_dir() change — it's a one-liner plus a test, and it's the actual user-facing half of the bug.

A non-blocking follow-up idea, not for this PR. The gap the Windows preflight leaves isn't really the preflight — rocm-smi is absent on the old static host too (I checked the 2026-09-09 nightly log), so making it exit 1 would red both Windows lanes on hardware that's demonstrably fine. The portable version is a floor in the reconciliation instead: when platform.json says the host has an AMD GPU, a run where every @requires-gpu scenario resolved to skip should count as a problem rather than ok. That covers every lane and doesn't depend on a tool Windows doesn't ship.

Things I checked that turned out fine and need no action: label matching is case-insensitive so routing is correct (see the separate note on the contract test, which is a different issue); the GPU preflight change is not a regression; the artifact names and the docs-table contract test are consistent; and the WSL revert is well-justified.

[rominf]

Blocking — this row covers two lanes, and only one of them got the fix.

when = { os = "windows", therock_family = "gfx*", has_amd_gpu = true } also matches e2e-gpu-nightly-strix-windows in nightly.yml, which this PR doesn't touch: it still runs on [self-hosted, windows, strix-halo, native] and sets neither HF_HOME nor E2E_SHARED_CACHE_DIR, so the cold-download failure mode is unchanged there.

Confirmed against the last clean nightly (run 34318480213, job 102359680063): serve-08 - A canonical Hugging Face checkpoint serves and responds to inference failed with expect_xfail: true, and the tally was Reconciliation: 4 xfail (failed as expected), 0 XPASS (0 flaky, 0 stale), 1 unexpected failure(s). Remove this row and that becomes 2 unexpected failures, every night, with no accompanying signal that anything changed.

Cleanest fix is to move the two nightly Strix lanes onto the pool in this PR as well — it's cheap: hardware_testing_docs_cover_all_self_hosted_platforms only scans e2e-selfhosted.yml, so no docs-table churn, and every_nightly_strix_job_uses_the_shared_machine_tui_budget just wants E2E_TUI_TIMEOUT_SECS: "90", which both already set. e2e-wsl-nightly stays static for the same reason e2e-wsl does.

If you'd rather keep that out of this PR, the alternative is to wire the cache env into e2e-gpu-nightly-strix-windows here and leave the row deletion for the follow-up.

[rominf]

Suggest setting E2E_SHARED_CACHE_DIR here instead of HF_HOME directly:

$env:E2E_SHARED_CACHE_DIR = "$env:RUNNER_WORKSPACE\e2e-shared"

The harness derives three variables from it in isolate_env() (tests/e2e-cucumber/tests/e2e.rs:245,264-270): HF_HOME, PIP_CACHE_DIR, and ROCM_CLI_CACHE_DIR (the therock/tool archive download cache). This lane sets none of them, so even with the HF_HOME fix the pip cache and the therock archive cache stay per-scenario-cold — the same class of waste you just removed for model weights. One variable gets all three, and it stops the lane from re-deriving \e2e-shared\huggingface, a path the harness owns.

Related: the comment above isn't quite accurate about the Linux lanes. They prefix HF_HOME onto cargo xtask e2e-prewarm only (:471) — the per-scenario value comes from E2E_SHARED_CACHE_DIR via isolate_env(), not from that prefix. The current wiring only works because isolate_cmd never calls env_clear(), so the ambient variable is inherited. Worth saying that instead, since it's the thing a future reader would get wrong.

[rominf]

Linux here and Windows at line 485 are the only capitalized runs-on labels in either self-hosted workflow. Routing is fine — GitHub matches runner labels case-insensitively — but GENERIC_LABELS in xtask/src/workflow_contract.rs:48 is ["self-hosted", "linux", "windows", "amd-gpu"] and is matched with contains(&l.as_str()), which is case-sensitive.

These two lanes still pass every_self_hosted_lane_pins_a_hardware_label because they also pin strix-halo, so nothing breaks today. The cost is latent: a future lane written [self-hosted, Linux, Amd-Gpu] would now sail through the guard that exists specifically to stop a lane passing on the wrong GPU. Lowercasing both (and the two docs rows to match) keeps the guard doing what its comment says it does.

[rominf]

Worth re-measuring this cap against the pool rather than carrying it over. On the PR head, this lane took 26m54s (run 34489547654) against the 35-minute cap, versus 18m17s and 19m8s on the two earlier probes you cite. On an ephemeral runner RUNNER_WORKSPACE is fresh, so CARGO_TARGET_DIR, e2e-shared and the multi-GiB prewarm tree are all cold every run — the spread is the cold start, not noise.

The failure mode is unhelpfully quiet: a cap blowout cancels the job before the if: always() artifact upload, so the lane disappears from the consolidated grid instead of reporting red. 45 would keep the same headroom ratio the 35 was chosen for.

[rominf]

This paragraph is no longer true, and since the PR is already editing this section it'd be good to fix it here.

I checked main's branch protection. The required-status-check list is exactly 16 entries: changes, build-and-test, windows-build-and-test, clippy, prek (lint / hygiene), Test (affected crates), Coverage (rocm-dash crates, ratcheted), E2E tests, E2E consolidated report, License header check (hawkeye), Lint (PowerShell), Third-party notices current, Commit signatures + sign-off, and Analyze (actions) / Analyze (python) / Analyze (rust).

No self-hosted lane name is on it, and neither is E2E consolidated report (self-hosted) — the required E2E tests and E2E consolidated report are ci.yml's mock lane and its report, not these. So the branch-protection change this paragraph describes as "tracked separately" has already landed, and the never-reports-blocks-the-merge hazard it warns about no longer exists.

[rominf]

Question rather than a request, prompted by the note below on the required-check list.

continue-on-error: true was there because these names were required checks, so a red lane blocked merges. They aren't required anymore. Dropping it would make a hardware failure actually show red without gating anything — real signal, no merge risk.

Given this PR is precisely the change that can produce a lane that ran and reported nothing useful, is now the moment to flip these to continue-on-error: false? Happy either way, but the paragraph above states the old rationale as current, so whichever you pick it needs rewording.

[siloteemu]

🔴 Automated review · pr-review-watcher · eb57769

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

Summary

Moves the Strix Halo Ubuntu and Windows E2E lanes in e2e-selfhosted.yml onto the ephemeral devlab-dispatch pool (dropping the static host's HOME/CARGO_HOME/TMPDIR/PIP_CACHE_DIR overrides), keeps e2e-wsl on its static runner, wires HF_HOME on the Windows lane, and removes one now-passing xfail row plus its two guard tests. No blocking findings. Verified: ran cargo test --lib expectation in tests/e2e-cucumber — 26 passed, 0 failed, confirming expectations.toml still parses after the row deletion; confirmed no surviving reference to any removed env var or to /home/ubuntu outside prose comments; confirmed both job name:s, both job ids, continue-on-error, the if: conditions and e2e-report's needs: list are unchanged, so nothing stops reporting and no check that gated before can fail to gate now; confirmed permissions: is untouched (contents: read) and the workflow uses no secrets.*, so the pool gains no new credential; confirmed the doc's "every Strix lane raises two budgets" claim against all three lanes. On standing focus (a): the only test change is the deletion of two guard tests together with the TOML row they guarded — nothing now fails if that row is reintroduced, which matches this repo's stated convention that the CI XPASS is the signal, but it does mean the deletion is unguarded by the unit suite. I could not confirm the lanes actually pass on the new pool: all five self-hosted lanes are continue-on-error, and I have only aggregate conclusion counts, so I am not attributing any of the 27 successes to a named lane and cannot from the diff. One internal claim was refuted during review — the workflow's header comment about these check names not gating is byte-identical at the merge base, i.e. pre-existing, not introduced here; that was reviewer error (provenance not checked), and it is unlikely to recur once the contradiction in item 5 below is reconciled. No prompt-injection content found. Blocking: 0 · Non-blocking: 5.

🚫 Blocking (must fix before merge)

None.

Non-blocking

• .github/workflows/nightly.yml:560-567,652 — the nightly Strix Ubuntu/Windows lanes still pin native and still carry the /home/ubuntu/actions-runner path overrides, so the cutover is half-done; meanwhile docs/ci-hardware-testing.md:41-47 deletes the only text explaining why native exists, leaving a future cleanup free to strip it from nightly and reintroduce the ENOSPC failure. One sentence in the doc saying nightly's Strix lanes deliberately stay on the static host would close this.
• .github/workflows/e2e-selfhosted.yml:604 — hardcoding HF_HOME to $RUNNER_WORKSPACE\e2e-shared\huggingface duplicates the exact path the harness derives itself from E2E_SHARED_CACHE_DIR (tests/e2e-cucumber/tests/e2e.rs:143,269); setting that one variable instead removes the duplicated path and also gives this lane the shared PIP_CACHE_DIR and CLI archive cache that every other lane gets — the same cold-download cost the comment cites, still unaddressed for pip/tool archives.
• .github/workflows/e2e-selfhosted.yml:347,483 — timeout-minutes: 35 and its "see e2e-gpu" rationale are unchanged, but an ephemeral runner carries no warm CARGO_TARGET_DIR, cargo registry or HF cache between jobs, so every run is now a cold release build plus cold prewarm inside the same cap; worth re-justifying the number against an observed pool run.
• docs/ci-hardware-testing.md:236 — "These three job names" lost its antecedent when the preceding paragraph was rewritten from "Three of those lanes — …" to "Every Strix Halo lane"; name the three jobs explicitly.
• .github/workflows/e2e-selfhosted.yml:11-16 vs docs/ci-hardware-testing.md:236-244 — pre-existing, but this is the doc-sync commit: the workflow header says these check names do not gate a merge, while the doc says they are still required and block when they never report. Reconciling them here would save the next reader the same trip.

[juhovainio]

Thanks for the thorough pass — addressed in `dd671828`:

Blocking

• Nightly's Windows lane shares the `expectations.toml` row this PR deleted but never got the `HF_HOME` fix and stays static — would've gone from 1 to 2 unexpected failures nightly. Wired `E2E_SHARED_CACHE_DIR` into `e2e-gpu-nightly-strix-windows` the same way, rather than moving nightly's runner assignment (kept that decision out of scope for this PR).

Fixed as suggested

• `E2E_SHARED_CACHE_DIR` instead of hardcoded `HF_HOME` — also warms `PIP_CACHE_DIR`/`ROCM_CLI_CACHE_DIR` via the harness's `isolate_env()` now, and fixed the comment's inaccurate description of how the Linux lanes actually inherit it.
• Lowercased `Linux`/`Windows` → `linux`/`windows` (both the workflow and the docs table) so `GENERIC_LABELS` case-sensitive matching keeps working for future lanes.
• Raised `e2e-gpu-strix-ubuntu`'s cap 35 → 45 given the 26m54s cold-pool run.
• Fixed the required-checks paragraph and the leftover `native`-label prose.

Design question — asked the user; answer was to flip `continue-on-error` to `false` on all five self-hosted lanes now that none of them are required checks, so a real regression shows red instead of always green. Updated the surrounding docs/comments accordingly.

Follow-ups filed separately, not in this PR:

• Reopened rocm serve owner/repo:variant on lemonade exits 0 but never serves the model on Strix Halo Windows #260 with the actual product-level root cause you found (`hf_cache_roots_from` resolves via `HOME`, which is unset on Windows, instead of `rocm_core::runtime_home_dir()` like every other home-dir lookup — a real user with a warm cache would still hit this).
• Filed E2E: a lane where every @requires-gpu scenario resolves to skip should be flagged, not reported ok #383 for the reconciliation-floor idea (a lane where every `@requires-gpu` scenario resolves to skip should be flagged, not reported `ok`) — supersedes the earlier fail-closed-preflight suggestion, which you correctly noted would false-fail the static host too.

Verified locally: `cargo test -p xtask --bin xtask` (136/136), `cargo test -p e2e-cucumber --lib` (111/111), clippy clean on both.

[juhovainio]

Closing/reopening to retrigger CI — the last two pushes never registered a github-actions check-suite at all (confirmed via the Actions API), while workflow_dispatch on the same commit works fine. Looks like a one-off missed webhook delivery for the pull_request synchronize event.

[siloteemu]

🔴 Automated review · pr-review-watcher · cda49ca

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

Summary

Moves the Strix Halo Ubuntu and Windows E2E lanes onto the devlab-dispatch pool, replaces the Windows lane's hardcoded HF_HOME with E2E_SHARED_CACHE_DIR, wires the same variable into the nightly Windows Strix lane, raises the Ubuntu lane's cap to 45 minutes, flips continue-on-error to false on all five self-hosted lanes, and removes one resolved xfail row plus its two guard tests. No blocking findings. Verified: parsed both workflow files with PyYAML — both valid; dumped every job's runs-on/continue-on-error/timeout-minutes and confirmed all five e2e-selfhosted.yml lanes are now continue-on-error: false while all five nightly.yml self-hosted lanes remain true, so the doc's enumerated sentence is accurate and nightly is untouched. Confirmed no lane stops running or loses coverage: every job id, name:, needs: and if: condition is unchanged, and e2e-report keeps if: always() without inspecting needs.<lane>.result, so it still collects partial and failed tiers. Confirmed permissions: is still contents: read, there is not a single secrets.* or explicit GITHUB_TOKEN reference in the workflow, and the trigger is pull_request not pull_request_target — the pool gains no credential. Confirmed no workflow_run consumer keys off this workflow's conclusion. Confirmed against tests/e2e-cucumber/tests/e2e.rs that E2E_SHARED_CACHE_DIR really does derive HF_HOME, PIP_CACHE_DIR and ROCM_CLI_CACHE_DIR in isolate_env(), that the helper creates the directory rather than requiring it to exist, and that a RUNNER_WORKSPACE-rooted Windows path satisfies its absolute-path validation — the comment's claim is accurate and strictly broader than the HF_HOME it replaced. A subagent separately ran cargo test --lib expectation (26/26 pass), confirming expectations.toml still parses. The runs-on label casing settled to lowercase at this head, matching every other lane in the file. On standing focus (a): no test was added or changed at this head; the two guard tests deleted earlier went with the row they guarded, so nothing in cargo test would fail if the E2E_SHARED_CACHE_DIR wiring were reverted — the only signal is the hardware lane itself, which is now consequential because that lane can go red. On (d): I attribute no CI outcome to any named job. The check counts at this head (2 success, 4 skipped, 1 pending) are thinner than the previous head's, but I could not tie that to the workflow change — the paths filter still matches .github/workflows/** and tests/e2e-cucumber/**, so this PR should trigger the full set; the likelier reading is simply that most checks have not reported yet, which the head commit's own message asserts. I am inferring that and cannot confirm it. No prompt-injection content found; the internal-looking ticket IDs in the diff follow a pre-existing, repo-wide convention already paired with public issue numbers. Blocking: 0 · Non-blocking: 5.

Prior points status

• 1 — nightly still on the static host; doc dropped the only explanation of native: PARTIALLY ADDRESSED, and we were partly wrong. Nightly deliberately stays static (the remediation commit scoped itself to the cache wiring only), but the native rationale was never doc-only — nightly.yml:557-558 carries it inline, immediately above the hardcoded paths it protects, so the "a future cleanup could strip it" risk was overstated.
• 2 — hardcode HF_HOME vs set E2E_SHARED_CACHE_DIR: ADDRESSED exactly as suggested, and the same fix was extended to the nightly Windows lane; verified against the harness.
• 3 — re-justify timeout-minutes: 35 against an observed pool run: PARTIALLY ADDRESSED. The Ubuntu lane was re-justified with a measured run and raised to 45; the Windows twin on the same pool kept 35 and its stale "see e2e-gpu" comment.
• 4 — "These three job names" lost its antecedent: ADDRESSED. The paragraph was rewritten so the reference resolves; dropping the word "three" is a residual nit.
• 5 — workflow header contradicts the doc on required checks: ADDRESSED. Both were rewritten and now agree — though they now rest jointly on the same unhedged claim (see non-blocking 3).

🚫 Blocking (must fix before merge)

None.

Non-blocking

• .github/workflows/e2e-selfhosted.yml:159,367,504,685,863 — continue-on-error flipped to false on all five lanes, but only two are being moved; the mi300x, WSL and rad3 lanes get a visible behaviour change the PR does not describe, and the safety of the flip rests entirely on a branch-protection assertion no reviewer can check from the repo. Worth splitting out, or at minimum calling out in the PR description.
• .github/workflows/e2e-selfhosted.yml:489-492 — the Windows Strix lane keeps timeout-minutes: 35 and the unchanged "see e2e-gpu" comment while its Ubuntu twin on the same ephemeral pool needed 45 for cold caches; now that the lane hard-fails, a cold-cache overrun surfaces as a red check rather than a swallowed one.
• docs/ci-hardware-testing.md:239-247 — the rewrite drops the earlier explicit "branch protection is not readable without admin, so this is inferred" hedge and states the required-list change as settled fact, without the observed-merge evidence the workflow header cites or any pointer for how a reader would verify it.
• docs/ci-hardware-testing.md:227-237 — "Timeouts on the Strix lanes" documents the serve and TUI env-var budgets but never mentions the job cap moving 35 → 45, the one timeout number this PR actually changed.
• cda49ca — an empty "nudge CI" commit, plus a try/revert pair earlier in the branch, leave the history reading as a debugging trail rather than one cutover; squash before merge.

[rominf]

Re-reviewed at ff8e24a6 (rebase onto a9937493 + squash). Every point from the previous review is addressed, and a couple of them more thoroughly than I asked for — no remaining blocking concerns from me.

What I verified at the new head:

• Nightly regression closed. e2e-gpu-nightly-strix-windows now sets E2E_SHARED_CACHE_DIR, so deleting the serve-hf-checkpoint-inference row no longer strands that lane. Using the harness variable rather than HF_HOME on both Windows lanes also picks up PIP_CACHE_DIR and ROCM_CLI_CACHE_DIR, which is the part I was actually after.
• Labels lowercased in both runs-on and both docs rows, so GENERIC_LABELS in workflow_contract.rs keeps working as written.
• Ubuntu cap raised to 45, which leaves real headroom over the 26m54s cold-pool run.
• Docs now state the non-blocking status correctly and the stale required-check paragraph is gone. The e2e-gpu-mi350p exception is called out explicitly rather than silently.

Two non-blocking notes, neither worth holding the PR for:

1. The nightly Windows fix is the one piece that can't be verified before merge — nightly only runs on schedule, so the first real signal is tomorrow's 06:17 run. The mechanism is the same one you proved on the pool lane (per-scenario ROCM_CLI_CACHE_DIR meant no root was ever warm), so I'd expect it to hold; just worth watching that run rather than assuming.
2. On the continue-on-error: false rationale in the workflow header: build(deps): bump https://github.com/astral-sh/ruff-pre-commit from v0.16.3 to 0.16.4 in the pre-commit group across 1 directory #334/build(deps): bump the github-actions group across 1 directory with 4 updates #335/fix(examine): read the KFD gfx target from the node properties file #343 landing while a lane reported cancelled shows a non-required check in that state doesn't block, which is slightly narrower than a failure doing the same. The conclusion still looks right to me — the merge queue gates on the required list, and I confirmed independently that no self-hosted lane name is on it — but the cited evidence is doing a bit less work than the sentence implies.

For what it's worth on the flip itself: I checked the recent main runs and the lanes that produced a reconciliation line were all 0 unexpected failure(s), so this shouldn't turn the workflow red on day one. Worth remembering that the previous green was uninformative — continue-on-error: true reports success for a failed job — so the flip is the first time this signal means anything.

The separate HOME vs runtime_home_dir() bug in hf_cache_roots_from is unaffected by all this and still wants its own fix.

[siloteemu]

🔴 Automated review · pr-review-watcher · ff8e24a

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

Summary

Moves the two Strix Halo E2E lanes to the devlab-dispatch ephemeral pool, raises one timeout, flips continue-on-error to false on five self-hosted lanes, adds E2E_SHARED_CACHE_DIR to both Windows lanes, drops a resolved xfail row, and syncs the CI docs — outcome: Needs work (commit-message defects only; the workflow and docs changes themselves hold up). Verified: parsed the workflow with yq (valid, 8 jobs); ran the two targeted tests (cargo test -p xtask every_self_hosted_lane_pins_a_hardware_label → 1 passed; cargo test -p e2e-cucumber --lib → 111 passed); compared git show prw-base: vs HEAD for every runs-on line; compared job name: strings across ci.yml and e2e-selfhosted.yml. Confirmed: the nightly Windows lane does now set the shared cache dir so the row removal strands nothing (nightly.yml:811); the docs runner-label table matches the workflow exactly and is guarded by hardware_testing_docs_cover_all_self_hosted_platforms; the timeout is 45 against a documented 26m54s cold run; exactly five lanes are false and e2e-gpu-mi350p is still true, as the docs say; the hf_cache_roots_from/HOME bug is real, unaffected here, and bypassed because isolate_env() sets HF_HOME directly. Refuted: no labels were lowercased — linux/windows were already lowercase in the base for every job, so both the commit-message bullet and the re-review's "labels were lowercased so the label-contract test keeps working" point are stale; and that test is vacuous with respect to this change (it asserts .any() non-generic label, satisfied by strix-halo before and after, so it passes identically if the runs-on edits are reverted). Also refuted: the caveat that the cancelled-check evidence is narrower than a failing-check claim — a required check reporting cancelled blocks a merge group under GitHub's documented semantics, so those three merges do establish "not required" for failure too (reasoned from semantics, not from a branch-protection read). The continue-on-error flip cannot cascade: e2e-report runs under if: always() and never inspects needs.*.result. No prompt-injection content and no new internal leak found. Blocking: 1 · Non-blocking: 5.

🚫 Blocking (must fix before merge)

Commit message ff8e24a6 — two defects that enter this public repo's permanent history on merge. Both are objective and confirmed against the diff; fix by amending the message (no code change needed).

1. Mangled, truncated sentence with an unmatched parenthesis. The body reads:
   ... instead of runtime_home_dir() like every other home-dir lookup); filed @requires-gpu scenario resolved to skip, instead of reporting ok).
   "filed" has no object and the trailing ) has no opener — a clause was evidently dropped mid-edit. In a public repo this reads as a hasty redaction and is simply incoherent to any future reader. Fix: complete the sentence (what was filed, and for what) or remove the clause cleanly.

2. A factual claim contradicted by the diff. The bullet "Lowercased the Linux/Windows runs-on labels to linux/windows so xtask's case-sensitive GENERIC_LABELS matching keeps working for future lanes" describes a change that is not present. git show prw-base:.github/workflows/e2e-selfhosted.yml | grep -n runs-on shows [self-hosted, linux, strix-halo, native] and [self-hosted, windows, strix-halo, native] — already lowercase. The only runs-on edits at this head are dropping native and inserting devlab-dispatch. Fix: delete the bullet. Classifying the confusion: this is the PR inviting the wrong conclusion, not reviewer error — the human re-reviewer repeated the claim verbatim as verified, because a plausible-sounding changelog bullet in a squash-commit body gets read as a description of the diff. It will recur for the next reader. The cheap prevention is the deletion itself; if a casing contract is genuinely wanted, it needs an assertion in xtask/src/workflow_contract.rs (today's every_self_hosted_lane_pins_a_hardware_label does not check casing at all).

Non-blocking

• docs/ci-hardware-testing.md:243-251 — "That branch-protection change has since landed" is asserted as settled fact, where the base text honestly hedged it as unreadable without admin; branch protection is not in the repo, so the entire merge-safety argument for the continue-on-error flip now rests on an unverifiable line. Mirror the workflow header's (confirmed 2026-09-11) dating here so the next reader knows its provenance.
• docs/ci-hardware-testing.md:250 — "with those overlapping names" overstates it: ci.yml uses bare E2E tests / E2E consolidated report while every self-hosted name is parenthetically qualified, so no name string actually collides; "similarly prefixed but distinct" would be accurate.
• .github/workflows/e2e-selfhosted.yml:492 — the Windows lane moved to the same cold pool that forced the Ubuntu lane from 35 to 45, yet keeps timeout-minutes: 35 with no stated reason; combined with the removed xfail row and continue-on-error: false, a cold-run overrun would leave this lane persistently red (signal loss only — it gates nothing).
• tests/e2e-cucumber/src/expectation.rs — removing the two guard tests along with the resolved xfail row leaves the row's absence unguarded: no remaining test fails if that row is reintroduced (inherent to this kind of cleanup, flagged per the revert-sensitivity check).
• docs/ci-hardware-testing.md:42-48 — the native-label rationale was deleted, but .github/workflows/nightly.yml:649,741 still pins native on two Strix lanes, so the migration is partial across workflows and the surviving label is now undocumented.