test(e2e): mirror the rocm-doctor skill and test its CLI contract

[volen-silo]

What

Mirrors the rocm-doctor skill (amd/skills#87) into skills/rocm-doctor/ and makes its documented contract executable, plus closes the unit-test gap around rocm fix's consent and exit-code behaviour.

rocm-doctor is a thin driver — the probe, the closed 15-mode catalog and the fixes all ship inside the rocm binary. What the skill owns is a contract: which fix-ids exist, which the CLI applies itself, which machines each is for, and what a diagnosis carries.

Nothing tested that seam. amd/skills has no rocm binary in CI, and diagnose.feature deliberately asserts only the shape of a diagnosis so it stays host-independent. A renamed fix-id or a 16th failure mode would merge green here and silently break the published skill.

Changes

Skill mirror — the three files byte-verbatim from amd/skills#87, sitting next to the binary they drive. walkthroughs/rocm-doctor.md is not copied; it is amd/skills-specific.

Contract feature — rocm_doctor_skill.feature (6 scenarios) parses reference.md's catalog table and diffs it against what rocm fix really reports, plus checks the diagnosis JSON and examine's status verdict are plumbed through. Query-only: no GPU, no serve, no download, no mutation, so it runs on the blocking mock lane and needs no capability tags.

This is a narrow, deliberate exception to the suite's black-box rule: nothing is imported from the codebase — a documentation artifact is read as test data, and that artifact is the thing under test.

rocm-core unit tests — exit codes 1, 4 and 5 were asserted nowhere, and no test reached a runner that writes to disk. The two that looked like they covered it did not: the dry-run test picks fix-2, whose Linux runner takes no FixOptions and never prompts, and the e2e preview scenario picks print-only fix-1, which returns before any runner. Ten tests now cover the consent gate, both mutation outcomes, and the dry-run short-circuit.

Licence headers — SKILL.md must open with YAML frontmatter for the skill loader, which pushes any header outside hawkeye's detection window. A positive exclude cannot override a hawkeye negation, so licenserc.toml now enumerates the markdown scopes to check and omits this one. Verified both directions: mirror exempt, a headerless docs/*.md still fails.

CI — skills/** joins the heavy paths filter (reference.md is now a fixture), and a non-blocking skill-mirror-drift job reports divergence from amd/skills in the step summary.

Scope

Claims already proven elsewhere are deliberately not restated: the exit codes and consent machinery are unit tested in rocm-core, the status enum in examine.rs, and the WSL2 out-of-scope rule in diagnose.rs. Two earlier scenarios were dropped for exactly this reason — one asserted an invariant that can only fire on a WSL2 host (no CI lane is WSL2), the other restated an existing exit-2 path.

Verification

• cargo xtask e2e — the 6 new scenarios pass; suite is otherwise unchanged
• cargo test --workspace --all-targets --exclude e2e-cucumber — clean
• cargo clippy --locked --workspace --all-targets and --locked -p e2e-cucumber --test e2e — both clean
• hawkeye check and all prek hooks — clean

Local WSL2 noise unrelated to this change: 2 diagnose scenarios (the catalog is out of scope on WSL2) and 2 proc_lifecycle tree tests.

Review follow-ups

Addressed from review of this branch:

• fix-9 contradicted itself on Linux (529f774). check_9_igpu_dgpu_collision reported auto_applicable: false on Linux while fix::RECIPES — what rocm fix dispatches on — says true; it was the only OS-split checker whose branches diverged. An agent branches on that field, so it was told fix-9 was print-only while rocm fix would happily apply it. Pre-existing, but this PR is what newly documents the value as tested, so it is fixed here rather than deferred. Guarded by a test that runs diagnose for both OS families and asserts every emitted fix agrees with the catalog; confirmed it fails with the old value.
• Phantom routing capability removed (529f774). route_when_no_match matched on lemonade / ollama / lm-studio and upstream_tracker carried those plus amdgpu-install, but Examination::probe only ever reports skipped, pytorch, llama-cpp or unknown — the arms were unreachable and the docs described a capability the CLI lacks. A companion test pins the reachable target set.
• Draft until Publish rocm-doctor skill amd/skills#87 merges, since several places here assert present-tense that these files mirror a published skill.

Still open, both upstream content that cannot be fixed without diverging the mirror — they belong in amd/skills#87:

• reference.md documents the lemonade / ollama / lm-studio routing removed above. SKILL.md hedges it ("only when the host probe detects that app") but the probe can never detect them, so both files need the correction.
• SKILL.md labels the shell installer "Linux / macOS" (install.sh has only a Linux arm) and says only nightly builds are published (v0.1.0-experimental.1 is tagged Latest, and install.sh already defaults to the release channel).

Notes

• Depends on Publish rocm-doctor skill amd/skills#87, which is still open. Until it merges the upstream files 404, so the drift job reports "not published yet" and passes — it is advisory and fails open by design.
• Review of this branch turned up two stale claims in the mirrored SKILL.md — it labels the shell installer "Linux / macOS" (install.sh is Linux x86_64 only) and says only nightly builds exist (v0.1.0-experimental.1 is published and install.sh already defaults to the release channel). Both are upstream content; fixing them here would diverge the mirror, so they belong in Publish rocm-doctor skill amd/skills#87.

[rominf]

Reviewed at 987dd9a2. Checked out locally: fix::tests 18/18 pass, all 22 CI checks green, and I re-verified the description's load-bearing claims (exit codes 1/4/5 were indeed unasserted before; the 15-mode catalog, OS scopes and auto-flags in reference.md match RECIPES exactly).

The refactor is the good part — splitting consent_without_prompt out of confirm and taking the rc path plus an injected consent verdict in pin_device_in_rc_file makes the mutation path genuinely testable without touching $HOME or stdin, and it's behaviour-preserving. The comments explaining why the two tests that looked like coverage weren't are worth keeping.

Three things I'd want resolved before this merges, then some smaller ones.

Dependent on an unmerged upstream PR, but not draft

The premise is that skills/rocm-doctor/ is byte-verbatim with amd/skills#87 — but #87 is still open and can still change in review. Merging first can land a mirror that never matches what upstream finally accepts, and the drift job fails open so nothing will say so. Suggest draft until #87 lands, then mirror the merged files.

That also sequences the next point for free.

The mirror ships documentation that's already known to be wrong

You flag it in the PR body, so this is agreement rather than a new find — but it interacts badly with the framing. The point of the change is that the documented contract becomes executable and can't silently break; a reader will reasonably assume the whole file is under test. It isn't: the new scenarios cover the catalog, and SKILL.md's install instructions are untested and currently false (install.sh:310-319 is Linux x86_64 only and hard-fails otherwise, against "Linux / macOS"; the nightly-only claim is likewise stale).

Fixing it upstream is the right layer. Landing #87 with the correction first and mirroring the corrected file avoids importing a known-wrong doc.

Scenario 2's pinned set duplicates an existing unit test, and its rationale doesn't hold

skill_steps.rs:264-266 justifies the hard-coded AUTO_APPLICABLE with "both files name these four ids in prose". SKILL.md contains no fix-ids at all — grep 'fix-[0-9]' skills/rocm-doctor/SKILL.md is empty. Only reference.md names them (line 43), and the assertion doesn't read that prose either: it compares the CLI to a Rust array, which is exactly what exactly_the_four_known_fixes_are_auto (fix.rs:1018) already asserts, same ids, same order.

So it's a duplicate by the PR's own "claims already proven elsewhere are not restated" standard. Either drop it, or make it do the thing the comment describes and parse the prose line out of reference.md.

Smaller

• world.model_name is reused to carry a symptom string (skill_steps.rs:184) and a fix-id (:196). The PR already adds a dedicated skill_reference field; these deserve the same rather than borrowing an unrelated name.
• pinning_without_a_device_index_is_not_applicable returns early on non-Linux, so it's a vacuous green on the Windows lane. #[cfg(target_os = "linux")] states that instead of passing silently.
• Both parsers panic! on an unrecognised row (skill_steps.rs:96, :123). Loud failure is defensible, but it means a cosmetic change to rocm fix output surfaces as a panic rather than an assertion diff naming the two sets.
• Scratch-dir cleanup is remove_dir_all(...).ok() after the assertions, so a failing test leaves debris under .rocm-work/. Minor, and the no-tempfile convention is a fair reason.
• !skills/rocm-cli-assistant/*.md is non-recursive, and a new top-level directory with tracked markdown goes unchecked silently. AGENTS.md documents the hazard, which is the right mitigation — worth being sure that note is the first thing anyone editing licenserc.toml sees.
• The drift job's if: github.event_name == 'pull_request' skips the merge queue. Harmless while it's advisory, but it means the last state before merge is never compared.

On the licence change specifically: I checked it positively rather than trusting the green check, since a passing hawkeye check only proves nothing is failing. Stripping README.md's header makes hawkeye flag it, so !/*.md anchoring works and the mirror is genuinely exempt — and no currently-tracked markdown lost coverage in the blanket-to-enumeration swap.