[rominf]

GitHub repointed ubuntu-latest from ubuntu-24.04 to ubuntu-26.04 (image ubuntu26/20261012.163). The last merge group to pass before the switch, #710, ran on 24.04; every merge-group run after it went red, with no change in this repository. Ubuntu 26.04 ships uutils coreutils, whose /bin/sleep is a multicall binary. The reclaim decoys were copies of it named llama-server, so they printed coreutils: unknown program 'llama-server' and exited at once. That failed preflight_runs_the_real_reclaim_assert_clear in Test (affected crates) and Coverage, and reclaim-gpu.sh --self-test in build-and-test. #712 fixed the decoys by copying bash and running a sleep 1 loop.

This PR builds on #712 in two ways.

1. Fork-free decoys that must be seen alive

• No copied binary and no child process. The xtask test runs /bin/sh with the decoy path as argv[0] (CommandExt::arg0). The shell waits in the read builtin on a stdin pipe the test holds. spawn_decoy in reclaim-gpu.sh runs its own bash the same way, via exec -a, and reads a FIFO it holds open for both reading and writing. Killing the one pid leaves nothing behind; fix(ci): keep the GPU reclaim decoys alive on multicall coreutils #712's loop left a sleep 1 child running for up to a second. The xtask decoy also exits by itself if the test dies before killing it, because the pipe closes. Running the real sleep under another argv[0] is not an option: uutils dispatches on argv[0] on some targets and on the executed path on others.
• Liveness checks. Before reading any decoy, both tests check it is alive. A dead decoy fails the test with a message naming it, instead of surfacing later as a gate or selection failure. The xtask test also checks that the decoy outlived the scan before it trusts a clear verdict. Run against the original cp /bin/sleep decoy on ubuntu-26.04, the self-test now opens with:

  FAIL: pre-warm decoy (pid=13) is not running; it exited at spawn, so the checks that read it test nothing
  

  and the xtask test with the decoy engine process (pid=…) exited at spawn (exit status: 1) ….

2. Linux CI on both LTS images instead of ubuntu-latest

Job	Images	Check names
build-and-test	24.04 + 26.04	build-and-test, build-and-test (ubuntu-26.04)
test	24.04 + 26.04	Test (affected crates), Test (affected crates) (ubuntu-26.04)
e2e	24.04 + 26.04	E2E tests, E2E tests (ubuntu-26.04)
remote-ssh	24.04 + 26.04	remote control channel (containerised), … (ubuntu-26.04)
every other hosted Linux job, in all workflows	24.04	unchanged

Required checks. Branch protection matches required checks by job name. The 24.04 leg of each split job therefore keeps its exact current name, using name: ${{ matrix.os == 'ubuntu-24.04' && '<name>' || format('<name> ({0})', matrix.os) }}. I checked that every required context is still produced by exactly one job. changes is the exception: it appears twice, once in ci.yml and once in e2e-selfhosted.yml, and was already duplicated on main. For an admin: to make 26.04 blocking, add these contexts as required:

• build-and-test (ubuntu-26.04)
• Test (affected crates) (ubuntu-26.04)
• E2E tests (ubuntu-26.04)

remote control channel (containerised) (ubuntu-26.04) is not required today, and neither is its 24.04 leg.

Single-LTS jobs. Each of these has a trailing comment on its runs-on saying why one image is enough:

• lint, metadata and docs jobs: changes, prek, license headers, lychee, Sphinx, commit signatures, skill checks, third-party notices, xfail hint, CodeQL;
• report merging and publishing jobs;
• clippy, which runs the same pinned toolchain on either image;
• coverage, which is one measurement against the committed floors (Test (affected crates) runs the tests on both images);
• the PR, nightly and release binary builds, which run in the manylinux container. Moving them off ubuntu-latest also keeps the image they run on from changing silently.

Caches keyed by image. rust-cache keys only on Linux-x64, and sccache keys on the compiler binary, which is the same rustup download on every image. Without the image in the key, a target directory or proc macro linked against 26.04's glibc would be restored on 24.04, and caches written by 26.04 runs under ubuntu-latest would be restored onto the jobs now pinned to 24.04. Every hosted Linux setup-rust-toolchain now passes cache-key with the image. The sccache prefixes become rocm-cli/build-and-test/<image>; clippy shares the 24.04 one. The coverage and cargo-about actions/cache keys now include ubuntu-24.04. As a result, the first run after merge starts with cold caches.

E2E gate. E2E tests used to run its scenarios only when build-and-test succeeded. That result now combines both images, so a failure on 26.04 alone would have let the required 24.04 leg report green without running anything. The gate now runs the scenarios whenever build-and-test reached a verdict.

E2E artifacts. The 24.04 leg still uploads e2e-report, which feeds E2E consolidated report. The 26.04 leg uploads ubuntu-26.04-e2e-results, which is for inspection only. The consolidated report has a single mock platform, and two inputs labelled as it would collide. Showing both images in that grid needs parse_descriptor support and is left as a follow-up.

Contract tests in xtask/src/workflow_contract.rs:

• no_workflow_runs_on_the_moving_ubuntu_latest_label scans every workflow file. moving_ubuntu_label_detector_flags_values_but_not_comments covers the detector itself.
• lts_split_jobs_keep_their_required_name_on_24_04 pins the exact name expression, runs-on: ${{ matrix.os }}, both images and fail-fast: false for the four split jobs.
• hosted_linux_compile_caches_are_keyed_by_runner_image checks that rust-cache cache-key, SCCACHE_AZURE_KEY_PREFIX and cargo actions/cache keys name the job's image.

I mutation-checked each test. Restoring ubuntu-latest on one job, using a plain matrix name, dropping 26.04 from the matrix, dropping fail-fast: false, and removing the image from a rust-cache key, the sccache prefix or the coverage key each made the matching test fail.

Testing

• Decoys: reclaim-gpu.sh --self-test (plain, and with a redirected TMPDIR) and preflight_runs_the_real_reclaim_assert_clear pass in ubuntu:24.04 (GNU coreutils), in ubuntu:26.04 (Ubuntu's uutils), and in ubuntu:24.04 with the uutils 0.12.0 release binary bind-mounted over /usr/bin/sleep. No decoy process is left running afterwards.
• cargo test -p xtask (321 passed), cargo fmt --check, both clippy invocations from CI with -D warnings, and cargo check -p xtask --tests --target x86_64-pc-windows-gnu.
• prek on the changed files passes, except for the hawkeye hook, whose local binary cannot parse licenserc.toml (a local version mismatch that also fails on main). The License header check (hawkeye) CI job covers it.
• actionlint 1.7.12 adds no new findings apart from label "ubuntu-26.04" is unknown. That release predates the label, which is listed in actions/runner-images. It already reported the self-hosted labels before this change.

This change affects CI and test fixtures only. CLI behaviour does not change, so no Gherkin scenario applies.

[jussielo-amd]

Nice fix — the FIFO-parked-shell decoy is a cleaner way to dodge the multicall-coreutils cmdline matching problem than the old fs::copy approach, and I confirmed the match logic (cmdline_matches_rule) only ever looks at /proc/<pid>/cmdline, never the file content, so dropping that copy step doesn't lose any coverage. Cleanup, the selftest_require_alive wiring across the other decoy-based fixtures, and the zombie fixture correctly staying unaffected all check out too.

One small thing I noticed, non-blocking: in the self-test liveness check (workflow_contract.rs around line 2868), there's a narrow TOCTOU window — if the decoy process exits between try_wait() and the /proc/<pid>/cmdline read, the code falls into the assert!(named, ...) branch and panics claiming the process "is alive but its command line does not start with {argv0}", when actually it just exited. It's a sub-millisecond race on an already-rare path, so it'd only ever mislead someone debugging a future flake rather than cause a false pass/fail — not something that needs to hold up this PR, but might be worth a follow-up if you want the panic message to stay accurate under that edge case.

Two things before this is mergeable, separate from the code review itself:

• GitHub currently shows this PR as having merge conflicts with the base branch — will need a rebase.
• The only check reporting here is the Read the Docs build; I don't see the usual cargo test/clippy gates on this PR, so I can't confirm they've actually run.

Approving on the logic/design itself — thanks for tracking down the root cause on ubuntu-26.04. Just flagging the conflict and CI status so they don't get missed before merge.

[rominf]

@jussielo-amd: fixed in ea4c3361. When the decoy's name doesn't match, it is now polled again, so a decoy that exits mid-check is reported as exited. The branch is rebased onto main, and CI has run on both images. Both E2E tests legs fail only on the two dashboard-quit scenarios that also fail on main (push run 39631143323). That failure is being investigated separately.

Stale: approved the earlier head. The head is now ea4c336: rebased onto main after #712, narrowed to what it adds on top of it, plus the ubuntu-24.04/26.04 CI split, verified locally and in CI. A fresh review is needed to merge.

[rominf]

Rebased onto main at 1c33248 to pick up #719's fix for the dashboard-quit timeout (timed out after 30s waiting for the TUI to exit after repeating "q"), which failed the previous E2E tests (ubuntu-26.04) run. The rebase was conflict-free and the branch's diff against main is line-for-line unchanged.

Verified locally on the new head 10fa805: cargo fmt --check, cargo test -p xtask (321 passed), cargo test -p rocm advised_commands, cargo clippy --workspace --all-targets -D warnings with and without --features rocm/e2e-test-hooks, the same clippy for x86_64-pc-windows-gnu, scripts/reclaim-gpu.sh --self-test (default and redirected TMPDIR) on the host and in ubuntu:24.04 and ubuntu:26.04 containers, shellcheck on that script, and the E2E job's "Decide whether to run" step executed against fixtures for each build-and-test result. actionlint 1.7.12 reports nothing new against main beyond not yet knowing the ubuntu-26.04 runner label.

[jussielo-amd]

Thanks for digging into this — the TOCTOU fix (ea4c3361) and merge-conflict state both check out: the panic path now re-polls when the cmdline doesn't match, and GitHub reports the branch mergeable against main (my earlier conflict note was from the 2026-10-21 state).

I don't think the E2E failures are pre-existing on main though. Both legs on this head (ea4c3361) fail dash-09 (dash-gen-tps-expiry-boundary) and dash-34 (dash-esc-menu-quit-row-confirms-when-serving) as unexpected regressions (ubuntu-24.04, ubuntu-26.04) — but I checked main's last four green runs and all four report 0 unexpected failure(s), with those two scenarios passing cleanly every time (latest). Main's only currently-tracked xfail failures are serve-01/serve-02, which are unrelated. So this looks like a regression specific to this branch rather than something already tracked — worth a look before merge.

Also: main has moved ~7 commits since the rebase, so it'll need a fresh one regardless.

[nowycondro]

🔴 Automated review · pr-review-watcher · 10fa805

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 — no blocking findings

Full review of the whole change (12 files, +517/-87).
Implements ROCMAI-670 ("CI: run Linux CI on both Ubuntu LTS images and make the GPU-reclaim test decoys fork-free") in full — the ticket's two stated goals (both-LTS CI, fork-free liveness-checked decoys) are exactly this PR's scope, nothing more or less.

Blocking

None

Non-blocking

None

Decisions for the author

• Matrix coupling delays required jobs on a slow/hung advisory leg — tradeoff
  build-and-test, test, e2e, and remote-ssh are each a single job id with a 2-image matrix (ubuntu-24.04 required, ubuntu-26.04 advisory), and every downstream needs: reads the job's aggregate result. A slow or hung 26.04 leg therefore delays the start of jobs gated on the required 24.04 leg, even though 26.04 can never fail the merge today. Splitting into separate job ids per image would remove the coupling at the cost of duplicating each ~20-line job definition four times over. Worth confirming the latency tradeoff was deliberate.
• windows-build-and-test/powershell-lint still run on windows-latest — non-blocking-improvement
  The same class of risk this PR fixes for Linux (a moving label silently repointing to a new OS image and breaking something, as ubuntu-latest → 26.04 did here) still exists on the Windows side. Out of this PR's stated scope, but worth a follow-up ticket.
• spawn_decoy_engine's /bin/sh (dash) does not respond to SIGTERM while blocked in read, unlike reclaim-gpu.sh's bash-based spawn_decoy — non-blocking-improvement
  Verified experimentally: bash's read builtin returns promptly on SIGTERM while blocked on the held-open FIFO; dash's does not (confirmed alive 6+ seconds; SIGKILL works immediately). This is invisible today — the Rust test's own cleanup always uses Child::kill() (SIGKILL) and neither test exercises the TERM→grace→KILL escalation path against this fixture — but a future author reusing spawn_decoy_engine to test that path would get a different signal story than the bash decoy gives. A one-line comment noting the asymmetry would save someone a surprising debugging session later.
• The FIFO ${path}.hold created by spawn_decoy is never explicitly removed — non-blocking-improvement
  Cleanup relies entirely on the scratch-tree rm -rf in self_test's EXIT trap. Harmless today since every caller in this file uses a path under a fresh mktemp -d tree, and it matches the existing pattern for the other decoy helpers' script files — just worth a short comment if a future caller ever reuses a fixed path across invocations.

Positive signals

• selftest_require_alive's six call sites earn their keep: reverting spawn_decoy to simulate the ubuntu-26.04 regression (immediate-exit decoy) was verified to turn a confusing downstream symptom (FAIL: pre-warm engine process was not selected) into the new, specific FAIL: pre-warm decoy (pid=...) is not running; it exited at spawn, so the checks that read it test nothing — exactly the diagnosability improvement the fixture rewrite is for.
• lts_split_jobs_keep_their_required_name_on_24_04 directly guards the exact branch-protection foot-gun a job-name matrix split risks (an off-by-one in the name expression would silently stop a required check from ever reporting again); it pins the literal name string, runs-on: ${{ matrix.os }}, both images, and fail-fast: false together.
• Cache-key re-keying by runner image was applied completely and consistently across all 8 workflow files (confirmed both by the new hosted_linux_compile_caches_are_keyed_by_runner_image contract test's logic and by independent manual enumeration of every cache-key/SCCACHE_AZURE_KEY_PREFIX/cargo actions/cache key in the diff); the one cargo-about cache shared between ci.yml and dependabot-manifests.yml was kept byte-identical between the two, as that job's own comment requires.
• Docs (docs/ci-hardware-testing.md) and the one affected doc comment (xtask/src/env_mutation_contract.rs) were updated in the same change and remain accurate against the new matrix shape — no stale ubuntu-latest mention survives anywhere in the touched files or the wider repo.
• The e2e gate's "ran to a verdict" vs. "succeeded" fix is correctly reasoned and correctly implemented: a one-leg build-and-test failure now still runs the required 24.04 e2e leg's scenarios instead of silently reporting green without having run anything, which was traced through all three relevant needs.build-and-test.result states (success, failure, cancelled) plus the workflow_dispatch short-circuit.

Deployment notes

• First run after merge starts every hosted-Linux compile cache cold (every cache key changed), as the PR's own testing section would say — expect one slower CI run before caches warm up again.
• At review time, two self-hosted hardware checks (E2E tests (W7900, current), E2E tests (MI300X, current)) are still pending; everything else (including both ubuntu-24.04 and ubuntu-26.04 legs of every split job) is green. Confirm these complete green before merging.
• Making the ubuntu-26.04 legs of build-and-test, Test (affected crates), and E2E tests actually block merges requires a separate admin action (adding their check names to branch protection's required list) — this PR's code change alone leaves 26.04 advisory-only.

What this covered

Read every one of the 12 changed files completely: scripts/reclaim-gpu.sh (full, both before/after comparison via git diff prw-base...pr-718), all 8 .github/workflows/*.yml files (full), docs/ci-hardware-testing.md (full), and in xtask/src/workflow_contract.rs and xtask/src/env_mutation_contract.rs the new/changed functions and tests plus their direct helper dependencies (not the files' full ~5700/~1600 lines, which hold hundreds of unrelated pre-existing tests — this was disclosed by each worker as a deliberate, bounded scope rather than a partial read of assigned material). Three independent blind fan-out workers covered: (1) the decoy-liveness bash+Rust logic, (2) ci.yml plus its own new contract tests, (3) every other touched workflow file, the docs, and the one doc-comment edit. history and code-comment-accuracy passes were run inline by each worker against their own files; agent-instruction adherence was checked against the repo's AGENTS.md by each group. The "prior changes" pass was run separately against PR #712 (the immediately-preceding, closely related fix this PR builds on) — its one non-blocking review note was already resolved by the author at the time and does not recur here.
Verification performed: live reproduction of the fork-free decoy's process/signal/no-children properties and of the dash-vs-bash SIGTERM difference; a before/after --self-test run simulating the ubuntu-26.04 regression to confirm selftest_require_alive's regression value; manual, exact-string tracing of lts_split_jobs_keep_their_required_name_on_24_04 against the pre-PR job names; manual enumeration of every compile-cache key across all 8 workflow files cross-checked against the new contract test's logic; a full-repo grep confirming no ubuntu-latest reference survives outside synthetic test fixtures in workflow_contract.rs. cargo test -p xtask itself could not be run — no Rust toolchain is available in this review environment — so the new xtask tests are verified by careful static tracing and targeted manual reproduction of their underlying shell/process mechanics rather than by executing them directly; this is the one check this review could not make.
Not reconciled against PR #718's own review comments or discussion thread — per this method's blind-review rule, that thread was deliberately not read at any point in this review.

[nowycondro]

🤖 Automated review (Claude Code) · 10fa805

I reviewed the full diff on the current head commit: scripts/reclaim-gpu.sh (the decoy-process rewrite), xtask/src/workflow_contract.rs and xtask/src/e2e_report.rs (CI-matrix/cache-key contract tests), .github/workflows/ci.yml plus the other workflow files touched, docs/ci-hardware-testing.md, and xtask/src/env_mutation_contract.rs. I did not find any blocking correctness issues. The shell-script rewrite closes the failure modes it targets (fork-free decoys, liveness checks right after each spawn, FIFO cleanup via the existing EXIT trap), and the new contract tests in workflow_contract.rs check out character-for-character against the actual YAML they assert on (e.g. the name: expression escaping and the deliberate exclusion of the ubuntu-26.04 artifact from the e2e- report glob).

Two minor, non-blocking, test-only observations on code duplication (no production/runtime impact): (1) xtask/src/workflow_contract.rs:779 — the new workflow_job_ids reimplements the same job-id extraction chain (top_level_block("jobs") + indent==2 + strip :) already inlined in the pre-existing self_hosted_e2e_jobs, rather than factoring it into one shared helper; a future fix to the filter logic applied to one copy could silently miss the other. (2) xtask/src/workflow_contract.rs:906 — the new hosted_linux_image_token calls flattened_values(job_block_text, "runs-on") directly instead of the existing runs_on_values wrapper (line 63) that does the same thing and carries a "continuation-safe" doc comment; if that wrapper is hardened further, this call site won't benefit.

I did not build or execute the shell script or xtask test suite in this sandbox (no GPU/ROCm hardware or CI runner available), so the review is based on static reading of the diff and surrounding code rather than execution.

🤖 Generated with Claude Code

[jussielo-amd]

Thanks for the work here — the fork-free decoy redesign and the dual-LTS CI split are both solid, and it's great to see every other behavioral change (the ubuntu-latest ban, job shapes, cache-key scoping) backed by a mutation-tested contract test in workflow_contract.rs.

One gap before this is mergeable: the e2e job's "Decide whether to run" step condition changed from needs.build-and-test.result == 'success' to also allow 'failure' — a real fix for the required leg reporting green without having run anything. But it's the one behavioral change here without a contract test, so a future edit could quietly narrow it back to 'success'-only and nobody would notice. Could you add a test alongside the others asserting it checks both 'success' and 'failure'?

Two small non-blocking notes:

• docs/ci-hardware-testing.md's "Blocking vs. non-blocking" section could mention the new ubuntu-26.04 legs aren't required checks yet (right now that's only in the PR description).
• The MI300X self-hosted failure on this head looks like an unrelated therock-next-09 scenario, and that lane isn't required — probably worth a rerun to confirm it's a flake, but not a blocker.

Requesting changes for the missing test; everything else looks good to me.