[rominf]

Fixes #607

What

Every bash GPU preflight (3 in e2e-selfhosted.yml, 5 in nightly.yml) computed deadline=$(( SECONDS + CEILING_SECS )) and checked it before the first poll. $SECONDS counts whole wall-clock seconds, so with a short ceiling the deadline could pass before the loop was ever entered. The step then reported a verdict that no reading had produced: "rocm-smi never returned within its timeout" (native) or "no VRAM figures under WSL" (advisory).

The test harness uses a 1 s ceiling, which made apu_preflight_gates_on_the_pool_the_engine_uses flaky under load. It has failed in CI once already. At the production ceiling of 90 s this was effectively unreachable, but nothing guaranteed it.

The loop is now:

first_poll=1
while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do
  first_poll=0
  …

So the deadline is checked only after a poll. The existing sleep 5; continue paths, backoff, messages and exit codes are unchanged. A while :; … break shape would not work: continue would skip a deadline check placed at the end of the body. The edit is identical in every copy, so the APU drift test stays green.

The PowerShell (Windows-native) preflight is left alone. It has the same check-before-first-poll shape, but because Get-Date is sub-second, skipping the first poll needs a real stall of at least 1 s rather than just crossing a second boundary. The 90 s production ceiling makes that unreachable.

Tests

• New regression test. every_shell_preflight_polls_at_least_once_whatever_the_ceiling extracts all 8 shell copies and runs each one under the shell CI uses. A BASH_ENV DEBUG trap stalls just over a second once the deadline is set, and each copy must still take a reading and fail on a held card.
  • On the old loop it fails with exactly the flaky message.
  • It also covers the discrete nightly.yml copies the drift test does not compare. Restoring the old loop in one of those alone fails this test.
• The holder-reporting path is now exercised. The harness runs each script from a temp dir containing a stub scripts/reclaim-gpu.sh, so the native lanes' --report-holders path is asserted. Before, the script was never found, so that failure path had never been tested.
• A signal kill is now named. A preflight killed by a signal now reports which signal, instead of "exited with a status code".
• A 7 s ceiling for apu_preflight_verdict_reflects_the_last_completed_reading. Its second poll now reliably runs; the first-poll change does not guarantee that.

Locally:

• cargo test --locked -p xtask passed 3/3 runs (224 tests; main has 223 since docs(architecture): rewrite citations as links, delete arch-doc checker #582 removed the architecture-doc checker).
• The APU preflight tests passed 20/20 under load.
• clippy -D warnings and cargo fmt --check are clean, and both workflows parse.

No Gherkin scenario: this is CI plumbing with no user-observable CLI behaviour.

Not in this PR:

• Bounding how many pwsh cases the test runs at once (a possible OOM signal kill).

• The advisory lane's absolute cd /root/work/rocm-cli, which keeps its holder report untestable in the harness.

• 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. (No rows reference ci: GPU preflight poll loop can run zero times, flaking apu_preflight_gates_on_the_pool_the_engine_uses #607 or the preflight.)

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

• Every new or changed user-facing message was read against the code path that runs after it, and its test asserts the resulting state, not only the wording, per AGENTS.md §3. (No CLI messages changed; the preflight verdicts are asserted together with exit codes and the holder-report side effect.)

[jussielo-amd]

🔴 Automated review · pr-review-watcher · 8614a9d

No blocking findings. Filed as an approving review at the requester's explicit
direction for this batch — see below for everything the review found.

Review — no blocking findings

Full review of the whole change (single commit 8614a9da, 3 files, base prw-base).
Fixes #607 entirely: the issue's primary suggested fix (unconditional first poll via a first_poll flag) is implemented in all 8 bash preflight copies, and its secondary/related items (signal-naming on exit, the untestable reclaim-gpu.sh path, and the apu_preflight_verdict_reflects_the_last_completed_reading ceiling that "silently exercises less") are all addressed too.

Blocking

None

Non-blocking

None

Decisions for the author

• The two untouched Windows-native PowerShell preflight loops (e2e-selfhosted.yml:753-760, nightly.yml:988-995) carry no in-code note explaining why they were left on the old while ((Get-Date) -lt $deadline) shape — non-blocking-improvement. The reasoning (sub-second Get-Date precision makes the race effectively unreachable there) is well explained in the PR description but absent from the code itself. A one-line comment at those two sites would keep a future reader (or someone extending apu_preflight_twins_do_not_drift-style drift checking to cross-check bash vs. PowerShell shape) from wondering whether this is a missed copy. Current behavior is correct either way; this is purely a documentation/maintainability suggestion.

Positive signals

• The regression test (every_shell_preflight_polls_at_least_once_whatever_the_ceiling) is unusually rigorous: a BASH_ENV DEBUG-trap mechanism deterministically reproduces the original race (guaranteed by sleeping 1.05s across a whole-second boundary) rather than relying on timing luck, and it drives all 8 real production script bodies extracted from the actual workflow files, not a reimplemented stand-in.
• The fix is applied as a mechanical, identical 6-line edit in all 8 copies rather than varying by call site, keeping the existing apu_preflight_twins_do_not_drift byte-for-byte drift guard meaningful.

Deployment notes

None — CI-only/test-only change, no runtime or release impact.

What this covered

Read both workflow YAML files in full and all of xtask/src/workflow_contract.rs's relevant regions (including run_preflight, run_preflight_pwsh, GateCase/assert_gate, gpu_preflight_steps/preflight_shell_script, and every "GPU preflight" step in both files) directly, plus two independent fan-out workers each read the full diff and surrounding context blind (no access to each other's output or to this driver's findings). Verified directly rather than taking claims on faith:

• Reverted just the two workflow files to the pre-fix loop shape and re-ran the new regression test: it fails with exactly the described message ("rocm-smi never returned within its timeout"), then reapplied the fix and confirmed it passes — genuine regression test, git status clean afterward (no residue).
• cargo test -p xtask --locked (full suite): 224/224 pass, matching the PR's own claim.
• cargo clippy -p xtask --all-targets --locked -- -D warnings: clean.
• Re-ran the three timing-sensitive preflight tests 5 times under --test-threads=4: no flakiness.
• Confirmed via line-by-line cross-reference of both yml files that all 8 bash loops (2 native + 1 advisory in e2e-selfhosted.yml; 4 native + 1 advisory in nightly.yml) got the identical patch, and the 2 pure-PowerShell native loops were correctly left alone.
• Confirmed CI status via gh pr view 608 --json statusCheckRollup: all ~30 checks green, including Test (affected crates), clippy, and Coverage (rocm-dash crates, ratcheted).
• Retrieved and cross-checked issue #607 and the PR description against the actual diff: no mismatch found between the description's claims and the code in either direction.

Did not run: cargo xtask coverage (not needed — only test code and a small helper were added, which can only raise coverage, not lower it); a dedicated "prior PR review comments on these files" pass (none found to be relevant via git log/git blame, which both fan-out passes checked as part of history review). Not reconciled against any discussion on this PR — per instructions, PR comments and prior reviews were not read. No prompt-injection attempts were found anywhere in the diff, commit message, code comments, or file contents.

[rominf]

The review's one remaining note asked for a code comment on the two PowerShell preflight loops, explaining why they keep their deadline-first shape. It is in #619. The comment is identical in both copies, so the drift check still compares them.