fix(diagnose): unify remediation flag wording with rocm fix

crates/rocm-core/src/diagnose.rs

@@ -1011,6 +1011,13 @@ fn check_9_igpu_dgpu_collision(e: &Examination, symptom: &str) -> Diagnosis {
             "Detected gfx targets: {gfx_targets:?}. Discrete GPU(s): {discrete_targets:?}; integrated APU(s): {apu_targets:?}. Pin HIP_VISIBLE_DEVICES to the discrete GPU — do not assume the higher-numbered gfx target is the dGPU (on RDNA3 the APU can be higher)."
         )
     };
+    // Marked auto_applicable below, but `rocm fix fix-9-igpu-dgpu` still needs
+    // --device-index to actually make the change: without it, both the Linux
+    // and Windows runners only print the query that finds the index and
+    // change nothing (see README's --device-index caveat).
+    let note = format!(
+        "{note} Without --device-index, `rocm fix` only prints this query and makes no change, despite being marked AUTO."
+    );
     let fix = if e.os_family == "windows" {
         Fix {
             summary: "Pin the HIP runtime to the discrete GPU with HIP_VISIBLE_DEVICES so the iGPU is hidden.".to_owned(),
@@ -1038,7 +1045,10 @@ fn check_9_igpu_dgpu_collision(e: &Examination, symptom: &str) -> Diagnosis {
                 "# Persist in your shell rc or your launch script.".to_owned(),
             ],
             fix_id: "fix-9-igpu-dgpu".to_owned(),
-            auto_applicable: false,
+            // Matches the `fix-9-igpu-dgpu` FixRecipe in fix.rs (auto_applicable:
+            // true, runner: run_hip_visible_devices) -- `rocm fix` can already
+            // carry this out on Linux, so the report must not claim otherwise.
+            auto_applicable: true,
             verify: "HIP_VISIBLE_DEVICES=1 python -c \"import torch; print(torch.cuda.device_count())\"".to_owned(),
             notes: vec![note],
             ..Fix::default()
@@ -2184,22 +2194,13 @@ pub fn render_report_text(report: &DiagnoseReport, top: usize) -> String {
             for c in &fix.commands {
                 let _ = writeln!(out, "     $ {c}");
             }
-            let mut flags = Vec::new();
-            if fix.needs_sudo {
-                flags.push("sudo");
-            }
-            if fix.needs_reboot {
-                flags.push("reboot required");
-            }
-            if fix.needs_relogin {
-                flags.push("re-login required");
-            }
-            if fix.auto_applicable {
-                flags.push("rocm fix can run it");
-            }
-            if !flags.is_empty() {
-                let _ = writeln!(out, "   flags: {}", flags.join(", "));
-            }
+            let flags = crate::fix::format_flags(
+                fix.needs_sudo,
+                fix.needs_reboot,
+                fix.needs_relogin,
+                fix.auto_applicable,
+            );
+            let _ = writeln!(out, "   flags: {}", flags.join(", "));
             for n in &fix.notes {
                 let _ = writeln!(out, "   note: {n}");
             }
@@ -2869,6 +2870,110 @@ mod tests {
             !note.contains("usually the higher-numbered"),
             "note must not repeat the old wrong gfx-number heuristic: {note}"
         );
+        assert!(
+            note.contains("Without --device-index") && note.contains("despite being marked AUTO"),
+            "note must warn that fix-9 is a no-op without --device-index: {note}"
+        );
+    }
+
+    #[test]
+    fn fix_9_igpu_dgpu_is_auto_applicable_on_linux() {
+        // `check_9_igpu_dgpu_collision`'s Linux/else branch sets
+        // `auto_applicable: true` to match the `fix-9-igpu-dgpu` FixRecipe in
+        // fix.rs (`run_hip_visible_devices` already handles it on Linux). This
+        // is a behavioural change, not text-only: it flips both the `Fix`
+        // struct field that `rocm diagnose --json` serialises and the
+        // `flags:` line `render_report_text` prints. Pin it directly so a
+        // regression back to `false` (the pre-fix value) fails here instead of
+        // only being visible by eyeballing output.
+        let mut e = linux_base();
+        e.has_apu = true;
+        e.has_discrete_amd = true;
+        e.gpus = vec![
+            Gpu {
+                gfx_target: "gfx1103".to_owned(),
+                is_amd: true,
+                is_apu: Some(true),
+                ..Gpu::default()
+            },
+            Gpu {
+                gfx_target: "gfx1100".to_owned(),
+                is_amd: true,
+                is_apu: Some(false),
+                ..Gpu::default()
+            },
+        ];
+        let report = diagnose(&e, "torch crashes with a segfault");
+        let hit = report
+            .matched
+            .iter()
+            .find(|d| d.id == "fix-9-igpu-dgpu")
+            .expect("iGPU+dGPU collision should be diagnosed");
+        let fix = hit.fix.as_ref().unwrap();
+        assert!(
+            fix.auto_applicable,
+            "fix-9-igpu-dgpu must be auto_applicable on Linux, matching the fix.rs catalog"
+        );
+
+        let text = render_report_text(&report, report.matched.len());
+        let lines: Vec<&str> = text.lines().collect();
+        let id_line = lines
+            .iter()
+            .position(|l| l.trim_start() == "id: fix-9-igpu-dgpu")
+            .expect("fix-9-igpu-dgpu should appear in the rendered report");
+        let flags_line = lines[id_line..]
+            .iter()
+            .find(|l| l.trim_start().starts_with("flags:"))
+            .expect("fix-9-igpu-dgpu should have a flags: line");
+        // Exact match, not `contains`: fix-9 carries only the auto flag, so a
+        // revert of the `render_report_text` call site to its pre-PR inline
+        // logic would still print a line containing "rocm fix can run it" and
+        // not "manual only" -- `contains` can't tell the two implementations
+        // apart. See `fix_11_iommu_rendered_flags_line_is_exact` for a fix-id
+        // whose optional flags actually differ between old and new wording.
+        assert_eq!(
+            flags_line.trim_start(),
+            "flags: rocm fix can run it",
+            "rendered flags: line for fix-9-igpu-dgpu: {flags_line}"
+        );
+    }
+
+    #[test]
+    fn fix_11_iommu_rendered_flags_line_is_exact() {
+        // Companion to `fix_9_igpu_dgpu_is_auto_applicable_on_linux`: that test
+        // only pins a fix-id with just the auto flag set, which an exact-match
+        // assertion can't distinguish from the pre-PR `render_report_text`
+        // inline logic (both print "rocm fix can run it" for it). fix-11-iommu
+        // carries sudo+reboot+manual, so this pins the full comma-joined,
+        // reworded `flags:` line through the real render call site.
+        let mut e = linux_base();
+        e.iommu_kernel_param = "on".to_owned();
+        e.gpus = vec![
+            Gpu {
+                is_amd: true,
+                ..Gpu::default()
+            },
+            Gpu {
+                is_amd: true,
+                ..Gpu::default()
+            },
+        ];
+        let report = diagnose(&e, "");
+        let text = render_report_text(&report, report.matched.len());
+        let lines: Vec<&str> = text.lines().collect();
+        let id_line = lines
+            .iter()
+            .position(|l| l.trim_start() == "id: fix-11-iommu")
+            .expect("fix-11-iommu should appear in the rendered report");
+        let flags_line = lines[id_line..]
+            .iter()
+            .find(|l| l.trim_start().starts_with("flags:"))
+            .expect("fix-11-iommu should have a flags: line");
+        assert_eq!(
+            flags_line.trim_start(),
+            "flags: requires sudo, requires reboot, manual only (`rocm fix` will NOT run it automatically)",
+            "rendered flags: line for fix-11-iommu: {flags_line}"
+        );
     }
 
     #[test]

crates/rocm-core/src/fix.rs

@@ -762,6 +762,45 @@ pub fn list_recipes() -> String {
     out
 }
 
+/// Canonical wording for a fix's remediation flags, shared by `rocm fix <id>`
+/// and `rocm diagnose` so the same `(sudo, reboot, relogin, auto_applicable)`
+/// values render as the same text from either command. This only
+/// standardizes wording, not the underlying values: `FixRecipe` (fix.rs) and
+/// diagnose's `Fix` still supply those independently, so a fix-id's rendered
+/// flags can still differ if the two disagree on a value (known example:
+/// `fix-5-amdgpu-load`'s `needs_reboot`). Also out of scope: the bare
+/// `rocm fix` catalog listing (`list_recipes`) describes the same
+/// `auto_applicable` property with a separate, untouched AUTO/PRINT-ONLY
+/// vocabulary.
+// These mirror the `FixRecipe`/`Fix` struct fields, where
+// `clippy::struct_excessive_bools` is already allowed workspace-wide; that
+// allow doesn't reach this free function's parameters, so
+// `clippy::fn_params_excessive_bools` is separately allowed below.
+#[allow(clippy::fn_params_excessive_bools)]
+pub(crate) fn format_flags(
+    needs_sudo: bool,
+    needs_reboot: bool,
+    needs_relogin: bool,
+    auto_applicable: bool,
+) -> Vec<&'static str> {
+    let mut flags = Vec::new();
+    if needs_sudo {
+        flags.push("requires sudo");
+    }
+    if needs_reboot {
+        flags.push("requires reboot");
+    }
+    if needs_relogin {
+        flags.push("requires re-login");
+    }
+    flags.push(if auto_applicable {
+        "rocm fix can run it"
+    } else {
+        "manual only (`rocm fix` will NOT run it automatically)"
+    });
+    flags
+}
+
 fn print_recipe(r: &FixRecipe) {
     println!("Fix:        {}  -- {}", r.fix_id, r.title);
     println!("OS scope:   {}", r.applies_on.join(", "));
@@ -772,22 +811,13 @@ fn print_recipe(r: &FixRecipe) {
             println!("  $ {c}");
         }
     }
-    let mut flags = Vec::new();
-    if r.needs_sudo {
-        flags.push("requires sudo");
-    }
-    if r.needs_reboot {
-        flags.push("requires reboot");
-    }
-    if r.needs_relogin {
-        flags.push("requires re-login");
-    }
-    if !r.auto_applicable {
-        flags.push("manual only (this command will NOT run it)");
-    }
-    if !flags.is_empty() {
-        println!("Flags:      {}", flags.join(", "));
-    }
+    let flags = format_flags(
+        r.needs_sudo,
+        r.needs_reboot,
+        r.needs_relogin,
+        r.auto_applicable,
+    );
+    println!("Flags:      {}", flags.join(", "));
     for n in r.notes {
         println!("Note:       {n}");
     }
@@ -1644,4 +1674,41 @@ mod tests {
         assert_eq!(apply("#1", &FixOptions::default()), 2);
         assert_eq!(apply("bogus", &FixOptions::default()), 2);
     }
+
+    #[test]
+    fn format_flags_covers_every_flag_combination_and_both_auto_states() {
+        // Exhaustive over all 2^4 = 16 combinations of the 3 optional flags
+        // (sudo/reboot/relogin) x both auto_applicable states, so a wording
+        // regression on any one flag, or on the always-present auto/manual
+        // marker, fails here rather than only being visible by eyeballing
+        // `rocm fix`/`rocm diagnose` output.
+        for bits in 0..16u8 {
+            let needs_sudo = bits & 1 != 0;
+            let needs_reboot = bits & 2 != 0;
+            let needs_relogin = bits & 4 != 0;
+            let auto_applicable = bits & 8 != 0;
+
+            let mut expected = Vec::new();
+            if needs_sudo {
+                expected.push("requires sudo");
+            }
+            if needs_reboot {
+                expected.push("requires reboot");
+            }
+            if needs_relogin {
+                expected.push("requires re-login");
+            }
+            expected.push(if auto_applicable {
+                "rocm fix can run it"
+            } else {
+                "manual only (`rocm fix` will NOT run it automatically)"
+            });
+
+            let flags = format_flags(needs_sudo, needs_reboot, needs_relogin, auto_applicable);
+            assert_eq!(
+                flags, expected,
+                "sudo={needs_sudo} reboot={needs_reboot} relogin={needs_relogin} auto={auto_applicable}"
+            );
+        }
+    }
 }

tests/e2e-cucumber/features/diagnose.feature

@@ -20,6 +20,7 @@ Feature: Diagnosing failures and listing fixes
     When the user asks the CLI to diagnose that symptom
     Then the CLI reports a likely cause with a suggested fix
     And every reported cause comes with a command that applies it
+    And every reported cause states its remediation flags
 
   @id:diagnose-always-offers-a-way-forward
   Scenario: diagnose-02 - Diagnosing any failure always gives the user a way to escalate
@@ -40,12 +41,17 @@ Feature: Diagnosing failures and listing fixes
     And each fix indicates whether the CLI can apply it automatically
     And the listing explains what those indicators mean
 
+  # This scenario exercises `rocm fix <id> --dry-run` (print_recipe's `Flags:`
+  # line), not the `rocm diagnose` report itself -- see diagnose-01's "states
+  # its remediation flags" step for the equivalent `flags:` line on that
+  # surface.
   @id:diagnose-fix-dry-run-changes-nothing
   Scenario: diagnose-05 - Previewing a fix explains the change without making it
     Given a user who has chosen a known fix
     When the user previews that fix without applying it
     Then the CLI describes what the fix would change
     And nothing on the machine is changed
+    And the preview states plainly that this fix is manual only
 
   @id:diagnose-fix-unknown-id-rejected
   Scenario: diagnose-06 - Asking for a fix the CLI does not know is refused clearly
@@ -266,3 +272,20 @@ Feature: Diagnosing failures and listing fixes
     When the user asks the CLI to diagnose that machine
     Then the CLI refuses and explains that it could not reach that machine
     And no diagnosis of this machine is reported
+
+  # diagnose-05 only proves the manual/zero-optional-flags wording, because
+  # PREVIEW_FIX_ID (fix-1-arch) needs none of sudo/reboot/re-login. The
+  # sudo+re-login combination only exists on a fix gated to bare-metal Linux
+  # (fix-4-render-group), so it needs its own scenario -- but, like
+  # diagnose-14, it is deliberately not OS-gated: `print_recipe` runs before
+  # the fix's own platform gate (see `apply` in fix.rs), so the Flags: text
+  # under test renders identically regardless of which lane runs it. The step
+  # asserts only that printed text, never the exit code -- `fix-4-render-group`
+  # gates its own dry-run on host state ($USER, `usermod`/`sudo` on PATH), so
+  # unlike PREVIEW_FIX_ID its exit code is not guaranteed to be 0 everywhere.
+  @id:diagnose-fix-preview-states-required-flags
+  Scenario: diagnose-20 - Previewing a fix that needs sudo and a re-login says so, and that it's auto-applicable
+    Given a user who has chosen a fix that needs sudo and a re-login
+    When the user previews that fix without applying it
+    Then the preview states that the fix requires sudo and a re-login
+    And the preview states that the CLI can run it automatically