feat(examine): show the AMD driver version on Windows and flag old drivers

README.md

@@ -286,7 +286,7 @@ form works depends on the engine your GPU selects.
 | `rocm diagnose --report` | Show what this machine would contribute to a problem report, and send nothing |
 | `rocm fix [<fix-id>]` | Apply a fix reported by `rocm diagnose` |
 | `rocm install sdk` | Install TheRock ROCm wheels into a managed Python environment |
-| `rocm install driver` | Install the AMD kernel driver on Linux |
+| `rocm install driver` | Install the AMD kernel driver on Linux; on Windows, report the display driver version and age (installs nothing) |
 | `rocm serve <model>` | Start a local OpenAI-compatible model server |
 | `rocm remote serve <machine> <model>` | Serve a model on another GPU machine over your private network |
 | `rocm dash` | Open the full-screen telemetry dashboard |
@@ -317,6 +317,20 @@ for its ROCm build and compiled GPU architectures:
 
 `--framework` only affects the JSON report, not the human-readable one.
 
+On Windows, the human-readable summary also shows the AMD display driver's
+`driver_version` and `driver_date`, and a `driver_age` line. `driver_age`
+compares the driver date with the release of the first HIP SDK for Windows
+(27 July 2023). AMD publishes no minimum driver version for the HIP SDK or for
+the Lemonade Vulkan backend, so this is a hint to look at the driver, not a
+statement that the driver is broken. The date is only a floor: it cannot judge
+newer GPUs such as gfx120X, gfx1150 and gfx1151, which may need newer drivers.
+`rocm diagnose` lists the same finding as
+`fix-22-display-driver-old` with a WEAK score. Its advice is AMD's support
+page and a restart after the install. The entry is advice about the host, not a
+cause of any reported symptom, so when nothing else matches, `rocm diagnose`
+still says where to file it upstream. rocm-cli never installs drivers on
+Windows. The Vulkan driver version is not shown.
+
 ### Diagnose and fix
 
 ```
@@ -554,6 +568,17 @@ you what it couldn't find instead.
 `install driver` installs the AMD kernel driver on Linux, using DKMS or a native
 package.
 
+On Windows, `install driver` is validate-only: it installs and changes nothing,
+with or without `--yes`, and it starts no driver command. It prints the AMD
+display driver version and date, and whether the date is older than the first
+HIP SDK for Windows, 27 July 2023 (`outdated: yes`, `no` or `unknown`). Next to
+`outdated: no` it repeats that the date cannot judge newer GPUs such as
+gfx120X, gfx1150 and gfx1151. For an
+old driver it also names the fix id `fix-22-display-driver-old`. Run
+`rocm diagnose` for the advice. AMD publishes no minimum driver version, so an
+old date is a hint to look at the driver, not a finding that it is broken. See
+[Examine](#examine).
+
 #### Updates
 
 `update` checks for a newer ROCm package.

apps/rocm/src/chat.rs

@@ -58,7 +58,7 @@ use std::process::Command as ProcessCommand;
 use std::time::Duration;
 
 #[derive(Debug, Clone, Eq, PartialEq)]
-enum ChatRocmCommandAction {
+pub(crate) enum ChatRocmCommandAction {
     ReadOnly(Vec<String>),
     Approval {
         args: Vec<String>,
@@ -2430,7 +2430,34 @@ fn render_services_tool_result_text(records: &[ManagedServiceRecord]) -> String
     output
 }
 
-fn chat_rocm_command_action_from_args(mut args: Vec<String>) -> Result<ChatRocmCommandAction> {
+/// The chat action for `rocm install driver`. Where the command only reports
+/// (`only_reports`, the same decision the command itself makes) it is a
+/// read-only call with no `--yes`; elsewhere it needs approval and carries
+/// `--yes`.
+///
+/// A model-supplied `--yes` is stripped in both forms, bare and `--yes=...`,
+/// as the `install sdk` arm does: the approval path appends its own bare
+/// `--yes`, so a stray attached form would otherwise ride along and clap would
+/// refuse the whole command, and the read-only path has no consent to grant.
+pub(crate) fn chat_install_driver_action(
+    mut args: Vec<String>,
+    only_reports: bool,
+) -> ChatRocmCommandAction {
+    args.retain(|arg| arg != "--yes" && !arg.starts_with("--yes="));
+    if only_reports {
+        return ChatRocmCommandAction::ReadOnly(args);
+    }
+    ensure_flag(&mut args, "--yes");
+    ChatRocmCommandAction::Approval {
+        args,
+        pending_title: "Install driver".to_owned(),
+        command_title: "Install".to_owned(),
+    }
+}
+
+pub(crate) fn chat_rocm_command_action_from_args(
+    mut args: Vec<String>,
+) -> Result<ChatRocmCommandAction> {
     canonicalize_chat_rocm_command(&mut args)?;
     validate_chat_rocm_command_safety(&args)?;
     let first = args.first().map(|value| value.to_ascii_lowercase());
@@ -2511,14 +2538,10 @@ fn chat_rocm_command_action_from_args(mut args: Vec<String>) -> Result<ChatRocmC
                 command_title: "Install".to_owned(),
             })
         }
-        Some("install") if second.as_deref() == Some("driver") => {
-            ensure_flag(&mut args, "--yes");
-            Ok(ChatRocmCommandAction::Approval {
-                args,
-                pending_title: "Install driver".to_owned(),
-                command_title: "Install".to_owned(),
-            })
-        }
+        Some("install") if second.as_deref() == Some("driver") => Ok(chat_install_driver_action(
+            args,
+            crate::driver_install::host_install_driver_only_reports(),
+        )),
         Some("update") if args.iter().any(|arg| arg == "--apply") => {
             ensure_flag(&mut args, "--yes");
             Ok(ChatRocmCommandAction::Approval {
@@ -3757,6 +3780,25 @@ fn run_rocm_read_only_in_process(paths: &AppPaths, args: &[String]) -> Result<St
         {
             render_setup_status_text(paths, &config)
         }
+        // `install driver` is a read-only call where the command only reports
+        // (`chat_install_driver_action`). It runs as a dry run that never
+        // executes anything, so a call that reaches it elsewhere cannot change
+        // the host either. `--dry-run` is accepted because it is what this arm
+        // already does; `--reconcile` is not, since it rewrites the saved driver
+        // install state and the command refuses to combine it with the other
+        // flags.
+        [command, subcommand, rest @ ..]
+            if command.eq_ignore_ascii_case("install")
+                && subcommand.eq_ignore_ascii_case("driver")
+                && rest.iter().all(|arg| {
+                    arg.eq_ignore_ascii_case("--dkms") || arg.eq_ignore_ascii_case("--dry-run")
+                }) =>
+        {
+            let dkms = rest.iter().any(|arg| arg.eq_ignore_ascii_case("--dkms"));
+            crate::driver_install::install_driver(paths, dkms, false, true)
+                .map(|result| result.output)
+                .map_err(|error| error.source)
+        }
         _ => bail!(
             "unsupported in-process read-only rocm command: {}",
             format_structured_tool_call("rocm", args)
@@ -6108,6 +6150,104 @@ model recipes
         );
     }
 
+    /// On Windows `rocm install driver` only reports, and chat plans it as a
+    /// read-only call. That call must run in process: before this arm existed it
+    /// failed with "unsupported in-process read-only rocm command". It is a dry
+    /// run, so it never executes anything on any platform.
+    #[test]
+    fn install_driver_read_only_call_runs_in_process() {
+        let (_root, paths) = test_paths("install-driver-in-process");
+        let args = vec!["install".to_owned(), "driver".to_owned()];
+        let text = run_rocm_read_only_in_process(&paths, &args)
+            .expect("the read-only install driver call should run in process");
+        assert!(text.contains("approval:"), "{text}");
+        assert!(!text.contains("driver install completed"), "{text}");
+        assert!(
+            run_rocm_read_only_in_process(
+                &paths,
+                &[
+                    "install".to_owned(),
+                    "driver".to_owned(),
+                    "--yes".to_owned()
+                ]
+            )
+            .is_err(),
+            "a flag the read-only path does not know is still refused"
+        );
+    }
+
+    /// `--dry-run` is what the in-process arm already does, so it is accepted
+    /// and the call reports a dry run. `--reconcile` writes driver install state
+    /// and stays refused, as does any flag the arm does not know.
+    #[test]
+    fn install_driver_read_only_call_accepts_dry_run_and_refuses_reconcile() {
+        let (_root, paths) = test_paths("install-driver-in-process-dry-run");
+        let run = |extra: &[&str]| {
+            let mut args = vec!["install".to_owned(), "driver".to_owned()];
+            args.extend(extra.iter().map(|arg| (*arg).to_owned()));
+            run_rocm_read_only_in_process(&paths, &args)
+        };
+        for extra in [&["--dry-run"][..], &["--dkms", "--dry-run"], &["--DRY-RUN"]] {
+            let text = run(extra).unwrap_or_else(|error| panic!("{extra:?} must run: {error}"));
+            assert!(text.contains("dry_run: true"), "{extra:?}: {text}");
+            assert!(
+                !text.contains("driver install completed"),
+                "{extra:?}: {text}"
+            );
+        }
+        for extra in [&["--reconcile"][..], &["--dry-run", "--reconcile"]] {
+            assert!(run(extra).is_err(), "{extra:?} must stay refused");
+        }
+        // Refusing `--reconcile` changed nothing on disk: the state file is
+        // written only by a reconcile or an executed install.
+        assert!(
+            !crate::driver_install::driver_install_state_path(&paths).exists(),
+            "a refused call must not create driver install state"
+        );
+    }
+
+    /// The `install driver` chat action strips `--yes` in the attached form too,
+    /// like the `install sdk` arm: read-only where the command only reports, and
+    /// a single bare `--yes` where it installs.
+    #[test]
+    fn chat_install_driver_strips_a_model_supplied_yes_in_both_forms() {
+        for supplied in ["--yes", "--yes=true", "--yes=1", "--yes=false"] {
+            let argv = |extra: &str| -> Vec<String> {
+                ["install", "driver", "--dry-run", extra]
+                    .iter()
+                    .map(|arg| (*arg).to_owned())
+                    .collect()
+            };
+            let ChatRocmCommandAction::ReadOnly(args) =
+                chat_install_driver_action(argv(supplied), true)
+            else {
+                panic!("a report-only platform must stay read-only");
+            };
+            assert_eq!(args, ["install", "driver", "--dry-run"], "{supplied}");
+
+            let ChatRocmCommandAction::Approval { args, .. } =
+                chat_install_driver_action(argv(supplied), false)
+            else {
+                panic!("an installing platform must ask approval");
+            };
+            assert_eq!(
+                args,
+                ["install", "driver", "--dry-run", "--yes"],
+                "{supplied}"
+            );
+        }
+        let ChatRocmCommandAction::ReadOnly(args) = chat_install_driver_action(
+            ["install", "driver", "--yes-not-a-flag"]
+                .iter()
+                .map(|arg| (*arg).to_owned())
+                .collect(),
+            true,
+        ) else {
+            panic!("read-only expected");
+        };
+        assert!(args.iter().any(|arg| arg == "--yes-not-a-flag"), "{args:?}");
+    }
+
     #[test]
     fn setup_status_renders_in_process() {
         let (_root, paths) = test_paths("setup-status-in-process");