feat(diagnose): offer a report the user sends themselves

README.md

@@ -296,6 +296,18 @@ fix` takes the id, not the position.
   The distribution is checked against a list of known names rather than
   repeated from the machine. Hardware that is not on AMD's published
   compatibility matrix produces no report at all, and the CLI says why.
+- `--send`, which requires `--report`, additionally offers a prefilled mail
+  carrying that report. It still sends nothing: the mail opens already filled
+  in with the content `--report` just printed, addressed to `ROCmCLI@amd.com`,
+  and it leaves the machine only when you send it yourself. Requiring
+  `--report` is what guarantees the content is shown before the mail is
+  offered. A mail client opens only when you asked and the machine looks like
+  a desktop you are at; over SSH, with no display, or with `ROCM_NO_BROWSER`
+  set, the address and the link are printed instead, which is also what
+  happens on a machine with no mail client. It is not combinable with
+  `--json`, which exists for scripts, and a script is not a person who can
+  read a mail before sending it. Note that a mail carries your address, which
+  the report itself does not.
 
 `fix` applies a known fix by the `id:` that `diagnose` reported — not the
 ranking position noted above, which isn't a stable name. Run it with no id

apps/rocm/src/comfyui.rs

@@ -6,6 +6,7 @@ use crate::cli_progress::AnimatedSpinner;
 use crate::{format_structured_tool_call, runtime_usability_status, therock};
 use anyhow::{Context, Result, bail};
 use flate2::read::GzDecoder;
+use rocm_core::browser::{Opener, SystemOpener};
 use rocm_core::{
     AppPaths, RocmCliConfig, download_file_to_path_with_progress, ensure_uv_binary,
     format_http_base_url, runtime_is_linux, runtime_is_windows, runtime_path_for_windows_child,
@@ -461,7 +462,7 @@ pub(crate) fn start(paths: &AppPaths, options: ComfyUiStartOptions) -> Result<St
     let browser_status = if options.no_open_browser {
         "not opened (--no-open-browser)".to_owned()
     } else {
-        match open_browser(&url) {
+        match SystemOpener.open(&url) {
             Ok(()) => "opened".to_owned(),
             Err(error) => format!("not opened ({error})"),
         }
@@ -1909,37 +1910,6 @@ fn child_path_string(path: &Path) -> String {
     }
 }
 
-fn open_browser(url: &str) -> Result<()> {
-    let status = if runtime_is_windows() {
-        Command::new("cmd")
-            .args(["/C", "start", "", url])
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status()
-    } else if cfg!(target_os = "macos") && !runtime_is_linux() {
-        Command::new("open")
-            .arg(url)
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status()
-    } else {
-        Command::new("xdg-open")
-            .arg(url)
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status()
-    }
-    .context("failed to open browser")?;
-    if status.success() {
-        Ok(())
-    } else {
-        bail!("browser opener exited with status {status}")
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

apps/rocm/src/main.rs

@@ -28,6 +28,7 @@ use crate::uninstall::uninstall;
 
 use anyhow::{Context, Result, bail};
 use clap::{CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum};
+use rocm_core::browser::Opener;
 use rocm_core::{
     AppPaths, AuditEventRecord, AutomationEventRecord, AutomationProposalRecord,
     AutomationRuntimeState, CodexBridgeEngine, CodexBridgeGpuSnapshot, CodexBridgeSnapshot,
@@ -165,6 +166,18 @@ enum Command {
         /// architecture check.
         #[arg(long, conflicts_with = "distro")]
         report: bool,
+        /// Also offer the prefilled issue form, so the report can be filed.
+        ///
+        /// Still sends nothing. This opens the form with the same content
+        /// `--report` printed, already filled in; it reaches the tracker only
+        /// when you submit it yourself. On a machine with no desktop, or one
+        /// reached over SSH, the link is printed instead of opened.
+        ///
+        /// Requires `--report`, so the content is always shown before the
+        /// form is offered. Not combinable with `--json`, which is for
+        /// scripts, and a script is not a person who can read a form.
+        #[arg(long, requires = "report", conflicts_with = "json")]
+        send: bool,
     },
     /// Apply a known fix by id (see `rocm diagnose`); run with no id to list fixes.
     ///
@@ -2078,7 +2091,8 @@ fn dispatch(cli: Cli) -> Result<()> {
             json,
             distro,
             report,
-        }) => diagnose(symptom, top, json, distro, report),
+            send,
+        }) => diagnose(symptom, top, json, distro, report, send),
         // Keep this error chained rather than discarding it into a fresh
         // `anyhow!(...)` (e.g. via a `.map_err` that restringifies it) -- see
         // `FixExitCode`'s doc comment for why that would silently break its
@@ -2741,6 +2755,7 @@ fn diagnose(
     json: bool,
     distro: Option<String>,
     report_requested: bool,
+    send: bool,
 ) -> Result<()> {
     // `rocm diagnose` is a query: it exits 0 whether it matched, found nothing,
     // or is out of scope. Callers read `has_match` / `out_of_scope` /
@@ -2775,7 +2790,7 @@ fn diagnose(
         .is_some_and(|wsl| !wsl.locally_probed);
     let report = rocm_core::run_diagnose(&examination, &symptom.unwrap_or_default());
     if report_requested {
-        return show_prepared_report(&examination, &report, json);
+        return show_prepared_report(&examination, &report, json, send);
     }
     if json {
         println!("{}", serde_json::to_string_pretty(&report)?);
@@ -2814,11 +2829,50 @@ fn established_entry(report: &rocm_core::DiagnoseReport) -> (Option<&str>, bool)
     })
 }
 
+/// Act on a delivery decision, and say what happened.
+///
+/// Takes the decision rather than making it, and takes the opener rather than
+/// being one. Both for the same reason: the decision is tested in `rocm-core`
+/// against every environment, and this half has to be tested against an opener
+/// that does not exist, on a machine with no browser. A function that decided
+/// and opened could be verified on neither.
+fn perform_delivery(
+    delivery: &rocm_core::report_delivery::Delivery,
+    opener: &dyn Opener,
+) -> String {
+    use rocm_core::report_delivery::{DESTINATION, Delivery};
+    match delivery {
+        // No mail client is started here on purpose, and the reason is worth
+        // the line: this is the branch for a machine held over SSH, or a
+        // server with no mail client at all, where starting one would open on
+        // somebody else's desktop or fail silently. The address is named as
+        // well as the link, because a machine in this state often cannot act
+        // on a `mailto:` at all and the user has to send the mail by hand.
+        Delivery::Show(url) => format!(
+            "Nothing has been sent. To send this yourself, mail the report above to \
+             {DESTINATION}, or open:\n  {url}"
+        ),
+        Delivery::Open(url) => match opener.open(url) {
+            Ok(()) => format!(
+                "Nothing has been sent yet. A prefilled mail to {DESTINATION} was opened, and \
+                 it is sent only when you send it:\n  {url}"
+            ),
+            // A failed open is not a failed command. The user still has the
+            // address and the link, which is the whole of what this offers.
+            Err(error) => format!(
+                "Nothing has been sent. A mail client could not be started ({error}). To send \
+                 this yourself, mail the report above to {DESTINATION}, or open:\n  {url}"
+            ),
+        },
+    }
+}
+
 /// Print the report this machine would contribute, and send nothing.
 fn show_prepared_report(
     examination: &rocm_core::Examination,
     report: &rocm_core::DiagnoseReport,
     json: bool,
+    send: bool,
 ) -> Result<()> {
     let (entry, fix_offered) = established_entry(report);
     // Exit 0 either way. A refusal is this command working, not failing: it
@@ -2835,7 +2889,17 @@ fn show_prepared_report(
                 println!();
                 println!("{}", serde_json::to_string_pretty(&prepared)?);
                 println!();
-                println!("Nothing has been sent. Sending is not implemented yet.");
+                // The content is printed above before this decides anything,
+                // so a report is always read before its form is offered. That
+                // ordering is the promise `--send` makes, and `--send`
+                // requires `--report` so it cannot be skipped.
+                let delivery = rocm_core::report_delivery::deliver(&prepared, send, &|key| {
+                    std::env::var(key).ok()
+                });
+                println!(
+                    "{}",
+                    perform_delivery(&delivery, &rocm_core::browser::SystemOpener)
+                );
             }
             Ok(())
         }
@@ -22029,6 +22093,113 @@ fn treat_as_natural_language(args: &[String]) -> bool {
 
 #[cfg(test)]
 mod tests {
+    use std::cell::RefCell;
+
+    use rocm_core::browser::Opener;
+    use rocm_core::report_delivery::Delivery;
+
+    use super::perform_delivery;
+
+    /// An opener that records rather than opens, and can be told to fail.
+    ///
+    /// The whole reason the opener is a trait: the real one spawns a browser
+    /// against whatever desktop exists, so neither "it was opened" nor "it was
+    /// deliberately not opened" can be observed in CI without this.
+    struct RecordingOpener {
+        opened: RefCell<Vec<String>>,
+        fails: bool,
+    }
+
+    impl RecordingOpener {
+        fn working() -> Self {
+            Self {
+                opened: RefCell::new(Vec::new()),
+                fails: false,
+            }
+        }
+        fn broken() -> Self {
+            Self {
+                opened: RefCell::new(Vec::new()),
+                fails: true,
+            }
+        }
+        fn opened(&self) -> Vec<String> {
+            self.opened.borrow().clone()
+        }
+    }
+
+    impl Opener for RecordingOpener {
+        fn open(&self, url: &str) -> anyhow::Result<()> {
+            self.opened.borrow_mut().push(url.to_owned());
+            if self.fails {
+                anyhow::bail!("no browser here");
+            }
+            Ok(())
+        }
+    }
+
+    /// Nothing is opened unless the decision was to open.
+    ///
+    /// The assertion that matters is on the opener, not on the wording. A
+    /// message saying no browser was started is satisfied by any string; an
+    /// opener that recorded nothing is the actual claim.
+    #[test]
+    fn a_delivery_that_is_not_an_open_never_reaches_the_browser() {
+        let delivery = Delivery::Show("mailto:nobody@example.invalid".to_owned());
+        let opener = RecordingOpener::working();
+        let said = perform_delivery(&delivery, &opener);
+
+        assert!(
+            opener.opened().is_empty(),
+            "a mail client was started for {delivery:?}, which is the one thing this path must \
+             not do on a machine the user is holding over SSH"
+        );
+        assert!(
+            said.contains("Nothing has been sent"),
+            "the user has to be told nothing left the machine: {said}"
+        );
+        // A machine in this state often cannot act on a `mailto:` at all, so
+        // the address has to be readable on its own, not only inside the link.
+        assert!(
+            said.contains(rocm_core::report_delivery::DESTINATION),
+            "a user who has to send the mail by hand needs the address: {said}"
+        );
+    }
+
+    /// Opening is what an open decision does, and the user is told it is not
+    /// filed yet.
+    #[test]
+    fn an_open_decision_reaches_the_browser_and_is_still_not_a_send() {
+        let url = "https://example.invalid/new?body=x";
+        let opener = RecordingOpener::working();
+        let said = perform_delivery(&Delivery::Open(url.to_owned()), &opener);
+
+        assert_eq!(
+            opener.opened(),
+            vec![url.to_owned()],
+            "premise failed: an open decision must reach the opener, otherwise the cases above \
+             are satisfied by never opening anything"
+        );
+        assert!(
+            said.contains("only when you send it"),
+            "opening a prefilled mail is not sending it, and the user has to know which one \
+             happened: {said}"
+        );
+    }
+
+    /// A browser that will not start still leaves the user the link.
+    #[test]
+    fn a_browser_that_fails_to_start_still_hands_the_user_the_link() {
+        let url = "https://example.invalid/new?body=x";
+        let said = perform_delivery(&Delivery::Open(url.to_owned()), &RecordingOpener::broken());
+
+        assert!(
+            said.contains(url),
+            "the link is the whole of what this offers, so a failed browser must not lose it: \
+             {said}"
+        );
+        assert!(said.contains("Nothing has been sent"));
+    }
 
     /// A diagnosis report holding exactly one finding.
     ///

crates/rocm-core/src/browser.rs

@@ -0,0 +1,67 @@
+// Copyright © Advanced Micro Devices, Inc., or its affiliates.
+//
+// SPDX-License-Identifier: MIT
+
+//! Handing a URL to the user's browser.
+//!
+//! Behind a trait because starting a browser is the one thing in this area
+//! that cannot run in CI: the real implementation spawns a process against
+//! whatever desktop the machine has, so any code path that opens a URL is
+//! untestable unless the opening itself can be replaced. Callers take
+//! `&dyn Opener`, tests pass a fake, and the decision to open stays separate
+//! from the opening.
+//!
+//! One implementation, deliberately. A second opener somewhere else is how the
+//! seam stops being a seam.
+
+use std::process::{Command, Stdio};
+
+use anyhow::{Context, Result, bail};
+
+use crate::{runtime_is_linux, runtime_is_windows};
+
+/// Something that can show the user a URL.
+pub trait Opener {
+    /// Hand `url` to the user's browser.
+    ///
+    /// # Errors
+    /// When no browser could be started, or the opener reported failure.
+    fn open(&self, url: &str) -> Result<()>;
+}
+
+/// The real one: whatever this platform uses to open a link.
+#[derive(Debug, Clone, Copy, Default)]
+pub struct SystemOpener;
+
+impl Opener for SystemOpener {
+    fn open(&self, url: &str) -> Result<()> {
+        let status = if runtime_is_windows() {
+            Command::new("cmd")
+                .args(["/C", "start", "", url])
+                .stdin(Stdio::null())
+                .stdout(Stdio::null())
+                .stderr(Stdio::null())
+                .status()
+        } else if cfg!(target_os = "macos") && !runtime_is_linux() {
+            Command::new("open")
+                .arg(url)
+                .stdin(Stdio::null())
+                .stdout(Stdio::null())
+                .stderr(Stdio::null())
+                .status()
+        } else {
+            Command::new("xdg-open")
+                .arg(url)
+                .stdin(Stdio::null())
+                .stdout(Stdio::null())
+                .stderr(Stdio::null())
+                .status()
+        }
+        .context("failed to open browser")?;
+        if status.success() {
+            Ok(())
+        } else {
+            bail!("browser opener exited with status {status}")
+        }
+    }
+}

crates/rocm-core/src/lib.rs

@@ -27,13 +27,15 @@ use windows_sys::Win32::System::Threading::{
     WaitForSingleObject,
 };
 
+pub mod browser;
 pub mod diagnose;
 pub mod disk_space;
 pub mod examine;
 pub mod fix;
 pub mod openmpi;
 pub mod proc_lifecycle;
 pub mod report;
+pub mod report_delivery;
 pub mod runtime;
 pub mod uv;
 pub use diagnose::{