diff --git a/README.md b/README.md index ab0c4b1ce..00fd31c97 100644 --- a/README.md +++ b/README.md @@ -482,7 +482,7 @@ updating a few times adds up. `rocm storage` shows where the space went and frees the parts that are safe to remove: ``` -rocm storage [report] [--json] +rocm storage [report [--json]] rocm storage remove-old-installs [--keep N] [--dry-run] [--yes] rocm storage remove-downloads [--dry-run] [--yes] ``` diff --git a/apps/rocm/src/advised_commands.rs b/apps/rocm/src/advised_commands.rs new file mode 100644 index 000000000..9db92201c --- /dev/null +++ b/apps/rocm/src/advised_commands.rs @@ -0,0 +1,1658 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Every `rocm …` / `rocmd …` command this repository tells a user to run must +//! exist and accept the flags it is given (AGENTS.md §3: remediation advice +//! naming a command must name one that parses). +//! +//! Example tests pin the *wording* of advice; nothing runs the advised command, +//! so a renamed flag or a removed subcommand strands users silently. This +//! module closes that gap mechanically: it walks every user-facing surface, +//! extracts each invocation it names, substitutes placeholders with values the +//! advice implies are valid, and routes the result through the same entry +//! points `run()` uses — the natural-language router, then the real clap tree. +//! +//! Surfaces scanned: +//! - the long `--help` of every visible command, rendered from the real clap +//! tree, so doc-comment help, `long_about` and `after_help` EXAMPLES are +//! checked at their source of truth; +//! - production Rust string literals under `apps/`, `crates/` and `engines/`: +//! backtick spans naming `rocm …`, and literals that *start* with a command +//! (the `rocm_core::fix` RECIPES `commands`/`verify` fields, dashboard +//! "Runs:" lines). Comments and `#[cfg(test)]` items are skipped — they are +//! never printed; +//! - `README.md`, `docs/**/*.md` and `skills/**/*.md` (inline backtick spans +//! and fenced code-block lines), and the VHS tapes under `docs/tapes/`. +//! +//! Out of scope by construction: extraction anchors on a leading `rocm ` / +//! `rocmd ` word, so commands for other tools (`apt`, `uv`, `amd-smi`, +//! `HIP_VISIBLE_DEVICES=…`) are never picked up. Contributor tooling +//! (`xtask/`, `crates/e2e-report/`, `tests/`) is not scanned: it is not shown to +//! users of the CLI. + +use std::fmt::Write as _; +use std::path::{Path, PathBuf}; + +use clap::FromArgMatches; +use clap::error::{ContextKind, ContextValue, ErrorKind}; + +use super::{ + Cli, cli_command, command_invocation_error, parse_freeform_invocation, should_treat_as_freeform, +}; + +/// What kind of text an invocation was found in. It decides whether leaving +/// out a required value is acceptable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Surface { + /// An inline backtick span in prose, which may name a command or flag + /// without its values ("pass `rocm serve --engine`"). + InlineProse, + /// A line the user is meant to run as written: a fenced code line, a + /// string literal that starts with a command (RECIPES `commands`/`verify`, + /// a dashboard `cmd`, a tape `Type`), a labelled `next step:`/`Try:`/ + /// `apply with:` line, or a help EXAMPLES row. + CommandLine, +} + +/// One advised invocation and where it came from. +#[derive(Debug, Clone)] +pub(crate) struct Advice { + pub source: String, + pub raw: String, + pub surface: Surface, +} + +/// How the real entry points treat an advised argv. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Verdict { + /// Parses into a structured command (or prints help/version). + Parses, + /// Names a real command or flag but leaves a required value off: a + /// *reference* to a command in prose ("pass `rocm serve --engine`"), not a + /// full command line. Every token it names exists, so it is not a + /// violation. + IncompleteReference, + /// Sent to the natural-language planner instead of clap. + Freeform, + /// clap rejects it: unknown subcommand or flag, invalid value, conflicting + /// flags, too many positionals. + Rejected(String), +} + +fn repo_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .ancestors() + .nth(2) + .expect("apps/rocm sits two levels below the repo root") + .to_path_buf() +} + +// --------------------------------------------------------------------------- +// Extraction +// --------------------------------------------------------------------------- + +fn starts_with_invocation(text: &str) -> bool { + let text = text.trim_start(); + text.starts_with("rocm ") || text.starts_with("rocmd ") +} + +/// Closed inline `` `rocm …` `` spans on one line. An unclosed trailing span is +/// ignored: callers join wrapped lines first. +fn backtick_spans(line: &str) -> Vec { + let parts: Vec<&str> = line.split('`').collect(); + let mut spans = Vec::new(); + // Parts at odd indexes are inside backticks; the last part is only closed + // when the split produced an odd number of parts. + let closed_inside = if parts.len() % 2 == 1 { + parts.len() + } else { + parts.len() - 1 + }; + for part in parts.iter().take(closed_inside).skip(1).step_by(2) { + let candidate = part.strip_prefix("$ ").unwrap_or(part); + if starts_with_invocation(candidate) { + spans.push(candidate.trim().to_owned()); + } + } + spans +} + +/// Unquoted invocations inside one string literal's content: +/// - the literal *starts* with a command — the `commands`/`verify` fields of +/// `rocm_core::fix` RECIPES, a dashboard `cmd: "rocm update"`, a VHS +/// `Type "rocm examine"`; a leading `#` (a commented-out command shown to +/// the user) is tolerated; +/// - a labelled line — `next step: rocm …`, `stop: rocm …`, `apply with: +/// rocm …`, `Try: rocm …` — the house style for remediation output; +/// - a command run over ssh — `ssh {target} -- rocm …`. +/// +/// Each `\n`-separated line of the literal is considered on its own. +fn literal_invocations(content: &str) -> Vec { + let mut found = Vec::new(); + for segment in content.split("\\n") { + // A commented-out command (`# rocm …`) keeps no indentation of its own. + let uncommented = segment.trim_start_matches('#'); + let start_stripped = if uncommented.len() == segment.len() { + segment + } else { + uncommented.trim_start() + }; + if let Some(command) = examples_row(start_stripped) { + found.push(command); + continue; + } + let mut search = 0; + while let Some(offset) = segment[search..].find("rocm") { + let at = search + offset; + search = at + 4; + let rest = &segment[at..]; + if !starts_with_invocation(rest) { + continue; + } + let before = segment[..at].trim_end(); + if before.ends_with(':') || before.ends_with(" --") || before.ends_with(" -- '") { + let command = rest.trim_end().trim_end_matches('\''); + found.push(command.to_owned()); + } + } + } + found +} + +/// Invocations named by the string literals on one source line. +fn command_literals(line: &str) -> Vec { + let bytes = line.as_bytes(); + let mut found = Vec::new(); + let mut index = 0; + while let Some(offset) = line[index..].find('"') { + let start = index + offset + 1; + let mut end = start; + let mut escaped = false; + while end < bytes.len() { + match bytes[end] { + b'\\' if !escaped => escaped = true, + b'"' if !escaped => break, + _ => escaped = false, + } + end += 1; + } + found.extend(literal_invocations(&line[start..end.min(line.len())])); + if end + 1 >= line.len() { + break; + } + index = end + 1; + } + found +} + +/// Production lines of a Rust file, with string continuations (`\` at end of +/// line) joined the way the compiler joins them. Comments and `#[cfg(test)]` +/// items are dropped: they are never printed to a user. +fn rust_production_lines(text: &str) -> Vec<(usize, String)> { + let lines: Vec<&str> = text.lines().collect(); + let mut kept: Vec<(usize, String)> = Vec::new(); + let mut continuing = false; + let mut index = 0; + while index < lines.len() { + let line = lines[index]; + if line.trim() == "#[cfg(test)]" { + // Skip the gated item by brace depth (or to its `;`). + let mut depth = 0i32; + let mut seen_open = false; + index += 1; + while index < lines.len() { + for ch in lines[index].chars() { + match ch { + '{' => { + depth += 1; + seen_open = true; + } + '}' => depth -= 1, + _ => {} + } + } + let ends_item = !seen_open && lines[index].trim_end().ends_with(';'); + index += 1; + if (seen_open && depth <= 0) || ends_item { + break; + } + } + continuing = false; + continue; + } + let is_comment = line.trim_start().starts_with("//"); + if continuing && !is_comment { + let (_, joined) = kept.last_mut().expect("a continued line exists"); + joined.pop(); // the trailing `\` + joined.push_str(line.trim_start()); + } else if !is_comment { + kept.push((index + 1, line.trim_end().to_owned())); + } + continuing = !is_comment && line.trim_end().ends_with('\\'); + index += 1; + } + kept +} + +fn extract_rust(path: &Path, rel: &str, out: &mut Vec) { + let text = std::fs::read_to_string(path).expect("read Rust source"); + for (line_no, line) in rust_production_lines(&text) { + let mut seen: Vec = Vec::new(); + let spans = backtick_spans(&line) + .into_iter() + .map(|raw| (raw, Surface::InlineProse)); + let literals = command_literals(&line) + .into_iter() + .map(|raw| (raw, Surface::CommandLine)); + for (raw, surface) in spans.chain(literals) { + if seen.contains(&raw) { + continue; + } + seen.push(raw.clone()); + out.push(Advice { + source: format!("{rel}:{line_no}"), + raw, + surface, + }); + } + } +} + +fn extract_markdown(path: &Path, rel: &str, out: &mut Vec) { + let text = std::fs::read_to_string(path).expect("read markdown"); + let mut in_fence = false; + // A fenced command continued with `\` (sh) or `` ` `` (PowerShell). + let mut pending: Option<(usize, String)> = None; + // Prose with an inline span wrapped across lines. + let mut prose: Option<(usize, String)> = None; + for (index, line) in text.lines().enumerate() { + let line_no = index + 1; + let trimmed = line.trim(); + if trimmed.starts_with("```") { + in_fence = !in_fence; + continue; + } + if in_fence { + let continued = trimmed.ends_with('\\') || trimmed.ends_with(" `"); + let piece = trimmed.trim_end_matches(['\\', '`']).trim_end(); + if let Some((start, mut joined)) = pending.take() { + joined.push(' '); + joined.push_str(piece); + if continued { + pending = Some((start, joined)); + } else { + out.push(Advice { + source: format!("{rel}:{start}"), + raw: joined, + surface: Surface::CommandLine, + }); + } + continue; + } + let command = piece + .strip_prefix("$ ") + .or_else(|| piece.strip_prefix("PS> ")) + .unwrap_or(piece); + if starts_with_invocation(command) { + if continued { + pending = Some((line_no, command.to_owned())); + } else { + out.push(Advice { + source: format!("{rel}:{line_no}"), + raw: command.to_owned(), + surface: Surface::CommandLine, + }); + } + } + continue; + } + let (start, joined) = match prose.take() { + Some((start, mut joined)) => { + joined.push(' '); + joined.push_str(trimmed); + (start, joined) + } + None => (line_no, line.to_owned()), + }; + // An odd backtick count means a span wraps onto the next line; a blank + // line ends the paragraph either way. + if joined.matches('`').count() % 2 == 1 && !trimmed.is_empty() { + prose = Some((start, joined)); + continue; + } + for raw in backtick_spans(&joined) { + out.push(Advice { + source: format!("{rel}:{start}"), + raw, + surface: Surface::InlineProse, + }); + } + } +} + +fn walk(dir: &Path, extensions: &[&str], skip: &[&str], files: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + let mut entries: Vec = entries.flatten().map(|entry| entry.path()).collect(); + entries.sort(); + for path in entries { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + if skip.contains(&name) { + continue; + } + if path.is_dir() { + walk(&path, extensions, skip, files); + } else if path + .extension() + .and_then(|e| e.to_str()) + .is_some_and(|ext| extensions.contains(&ext)) + { + files.push(path); + } + } +} + +/// Every advised invocation on a source surface. Help text is collected +/// separately by [`help_text_advice`]. +pub(crate) fn source_advice() -> Vec { + let root = repo_root(); + let rel = |path: &Path| { + path.strip_prefix(&root) + .unwrap_or(path) + .to_string_lossy() + .replace('\\', "/") + }; + let mut advice = Vec::new(); + + // `tests` directories and `*tests.rs` files are test code; `target` is + // build output; `e2e-report` renders CI reports for contributors. + let rust_skip = ["target", "tests", "e2e-report", "benches"]; + let mut rust_files = Vec::new(); + for top in ["apps", "crates", "engines"] { + walk(&root.join(top), &["rs"], &rust_skip, &mut rust_files); + } + for path in rust_files { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + // This module's own fixtures are not advice. + if name.ends_with("tests.rs") || name == "build.rs" || name == "advised_commands.rs" { + continue; + } + extract_rust(&path, &rel(&path), &mut advice); + } + + let mut markdown = vec![root.join("README.md")]; + walk(&root.join("docs"), &["md"], &[], &mut markdown); + walk(&root.join("skills"), &["md"], &[], &mut markdown); + for path in markdown { + extract_markdown(&path, &rel(&path), &mut advice); + } + + let mut tapes = Vec::new(); + walk(&root.join("docs").join("tapes"), &["tape"], &[], &mut tapes); + for path in tapes { + let text = std::fs::read_to_string(&path).expect("read tape"); + for (index, line) in text.lines().enumerate() { + if line.trim_start().starts_with('#') { + continue; + } + for raw in command_literals(line) { + advice.push(Advice { + source: format!("{}:{}", rel(&path), index + 1), + raw, + surface: Surface::CommandLine, + }); + } + } + } + advice +} + +/// The command on a line that starts with one. An *indented* line is a row of +/// an EXAMPLES table (` rocm examine Check GPU …`): clap renders a +/// description column after a run of spaces, so the command ends at the first +/// double space. Only there: on any other line a double space is ordinary +/// whitespace inside a command, and cutting at it would hide what follows +/// (`rocm install sdk --channel release --bogus-flag`). +/// +/// The rows are found twice, by design: in the rendered `--help`, and in the +/// `after_help` string literal they are written in. +fn examples_row(line: &str) -> Option { + let trimmed = line.trim(); + if !starts_with_invocation(trimmed) { + return None; + } + let indented = line.starts_with(char::is_whitespace); + let command = match trimmed.split_once(" ") { + Some((command, _)) if indented => command, + _ => trimmed, + }; + Some(command.trim_end().to_owned()) +} + +/// Every invocation named in the long `--help` of every visible command. +pub(crate) fn help_text_advice() -> Vec { + fn visit(command: &mut clap::Command, path: &str, out: &mut Vec) { + let help = command.render_long_help().to_string(); + for (index, line) in help.lines().enumerate() { + let source = format!("`{path} --help` line {}", index + 1); + if let Some(raw) = examples_row(line) { + out.push(Advice { + source: source.clone(), + raw, + surface: Surface::CommandLine, + }); + } + for raw in backtick_spans(line) { + out.push(Advice { + source: source.clone(), + raw, + surface: Surface::InlineProse, + }); + } + } + // clap's generated `help` subcommand renders the same help again; one + // bad line would be reported once per nesting level. + let names: Vec = command + .get_subcommands() + .filter(|sub| !sub.is_hide_set() && sub.get_name() != "help") + .map(|sub| sub.get_name().to_owned()) + .collect(); + for name in names { + let sub = command + .find_subcommand_mut(&name) + .expect("subcommand listed above"); + visit(sub, &format!("{path} {name}"), out); + } + } + let mut root = cli_command(); + root.build(); + let mut out = Vec::new(); + visit(&mut root, "rocm", &mut out); + out +} + +// --------------------------------------------------------------------------- +// Normalisation: from an advised string to argv variants +// --------------------------------------------------------------------------- + +/// Split a shell list into its commands at `&&`, `||`, `;` and `|`. A +/// separator inside quotes, a `<…>` placeholder, a `[…]` optional group or a +/// `{…}` template is notation, not a separator, and so is a `|` joined to a +/// word (`stop|restart`): a pipe stands alone between spaces. +fn split_shell_list(text: &str) -> Vec { + let chars: Vec = text.chars().collect(); + let mut commands = Vec::new(); + let mut current = String::new(); + let mut quote: Option = None; + let mut closers: Vec = Vec::new(); + let mut index = 0; + while index < chars.len() { + let ch = chars[index]; + let previous = index.checked_sub(1).map(|i| chars[i]); + let next = chars.get(index + 1).copied(); + let at_word_start = previous.is_none_or(char::is_whitespace); + if let Some(q) = quote { + if ch == q { + quote = None; + } + current.push(ch); + index += 1; + continue; + } + let separator_len = match (ch, next) { + _ if !closers.is_empty() => 0, + ('&', Some('&')) | ('|', Some('|')) => 2, + (';', _) => 1, + ('|', _) if at_word_start && next.is_none_or(char::is_whitespace) => 1, + _ => 0, + }; + if separator_len > 0 { + commands.push(std::mem::take(&mut current)); + index += separator_len; + continue; + } + match ch { + '"' | '\'' => quote = Some(ch), + // `<` opens a placeholder only at a word start and before a + // non-space: ` < file` is a redirection. + '<' if at_word_start && next.is_some_and(|c| !c.is_whitespace()) => closers.push('>'), + '[' => closers.push(']'), + '{' => closers.push('}'), + c if closers.last() == Some(&c) => { + closers.pop(); + } + _ => {} + } + current.push(ch); + index += 1; + } + commands.push(current); + commands +} + +/// The commands an advised string names, each cut down to the command +/// itself: a shell comment or escaped newline ends the whole line; each +/// command of a shell list is kept when it runs `rocm`/`rocmd` (commands for +/// other tools are not this contract's business) and is then stripped of +/// redirections and trailing prose. +pub(crate) fn command_parts(raw: &str) -> Vec { + let mut text = raw.trim(); + for marker in ["\\n", " # "] { + if let Some(position) = text.find(marker) { + text = &text[..position]; + } + } + let mut parts = Vec::new(); + for command in split_shell_list(text) { + let command = command.trim(); + if !(starts_with_invocation(command) || command == "rocm" || command == "rocmd") { + continue; + } + let mut command = command.to_owned(); + for marker in [ + " >> ", " > ", " < ", " 2>", " (", " —", " –", " → ", " before ", " then ", + ] { + if let Some(position) = command.find(marker) { + command.truncate(position); + } + } + let command = command.trim().trim_end_matches([',', ':']).trim(); + if !command.is_empty() { + parts.push(command.to_owned()); + } + } + parts +} + +/// Shell-like word split honouring single and double quotes. A `<…>` +/// placeholder is one word even when it contains spaces +/// (``). +fn split_words(text: &str) -> Vec { + let mut words = Vec::new(); + let mut current = String::new(); + let mut quote: Option = None; + let mut has_word = false; + for ch in text.chars() { + match (quote, ch) { + (Some('>'), '>') => { + current.push('>'); + quote = None; + } + (Some(q), c) if c == q => quote = None, + (Some(_), c) => current.push(c), + (None, '"' | '\'') => { + quote = Some(ch); + has_word = true; + } + (None, '<') if current.is_empty() || current.ends_with(['[', ':', '#']) => { + current.push('<'); + quote = Some('>'); + } + (None, c) if c.is_whitespace() => { + if has_word || !current.is_empty() { + words.push(std::mem::take(&mut current)); + has_word = false; + } + } + (None, c) => current.push(c), + } + } + if has_word || !current.is_empty() { + words.push(current); + } + words +} + +/// The value a placeholder stands for. `previous` is the word before it, which +/// disambiguates bare `{}` / `` placeholders. Values are ones the advice +/// implies are valid: a recognised TheRock family, a real engine, a real shell. +pub(crate) fn placeholder_value(name: &str, previous: &str) -> String { + let key = name + .trim_matches(['<', '>', '{', '}']) + .trim() + .to_ascii_lowercase(); + let by_previous = match previous { + "fix" => Some("fix-1-arch"), + "--distro" => Some("ubuntu"), + "--engine" => Some("vllm"), + "--channel" => Some("release"), + "--format" => Some("wheel"), + "--family" => Some(rocm_core::known_therock_families()[0]), + "--provider" => Some("openai"), + "--runtime" | "--runtime-id" | "activate" | "uninstall" => Some("runtime-key-1"), + "--service" | "stop" | "restart" | "logs" | "remove" => Some("svc-1"), + "--mode" => Some("propose"), + "--endpoint" => Some("http://127.0.0.1:8000"), + "--replay" => Some("/tmp/replay.json"), + "--keep" | "--top" | "--device-index" | "--older-than-hours" => Some("1"), + "--concurrency" => Some("1,2"), + "--artifact-max-bytes" => Some("1048576"), + "--local-webhook-port" | "--port" => Some("8080"), + "--host" => Some("127.0.0.1"), + "--version" => Some("7.0.0"), + "--yes" => Some("start a local model"), + "completions" => Some("bash"), + "doctor" => Some("box-1"), + _ => None, + }; + if let Some(value) = by_previous { + return value.to_owned(); + } + let value = match key.as_str() { + k if k.contains("family") => rocm_core::known_therock_families()[0], + "model" | "base" | "owner/repo" => "Qwen/Qwen3-0.6B", + "quant" => "Q4_0", + k if k.contains("fix") => "fix-1-arch", + k if k.contains("key") || k.contains("runtime") || k == "candidate" => "runtime-key-1", + k if k.contains("service") || k.contains("session") || k == "id" => "svc-1", + "engine" => "vllm", + "shell" => "bash", + "url" => "http://127.0.0.1:8000", + "machine" | "target" => "box-1", + "provider" | "name" => "openai", + "port" => "8080", + "version" => "7.0.0", + "host" => "127.0.0.1", + "mode" => "propose", + "watcher" => "server-recover", + "n" | "bytes" => "1", + "file" | "missing" => "/tmp/replay.json", + "command" | "args" => "examine", + k if k.contains("request") || k.contains("text") || k.contains("error") => { + "start a local model" + } + _ => "x1", + }; + value.to_owned() +} + +/// Whether a bare word is synopsis notation for a value (`URL`, `NAME`, `N`). +fn is_upper_placeholder(word: &str) -> bool { + word.chars() + .all(|c| c.is_ascii_uppercase() || c == '_' || c == '-') + && word.chars().any(|c| c.is_ascii_uppercase()) +} + +fn substitute_placeholders(word: &str, previous: &str) -> String { + if is_upper_placeholder(word) + || word.starts_with("N,") + || (word.starts_with('<') && word.ends_with('>')) + || (word.starts_with('{') && word.ends_with('}')) + { + return placeholder_value(word, previous); + } + // Embedded placeholders: `:`, `svc-{id}`. + let mut out = String::new(); + let mut rest = word; + while let Some(open) = rest.find(['<', '{']) { + let close_char = if rest.as_bytes()[open] == b'<' { + '>' + } else { + '}' + }; + let Some(close) = rest[open..].find(close_char) else { + break; + }; + out.push_str(&rest[..open]); + out.push_str(&placeholder_value(&rest[open..=open + close], previous)); + rest = &rest[open + close + 1..]; + } + out.push_str(rest); + out +} + +/// Whether a word is a placeholder for options the advice spells out elsewhere +/// (``). It stands for zero or more flags the +/// reader picks, so it contributes no words: the command around it is still +/// checked, rather than failing on an invented value for a prose placeholder. +fn is_options_placeholder(word: &str) -> bool { + word.starts_with('<') && word.ends_with('>') && word.to_ascii_lowercase().contains("options") +} + +fn is_ellipsis(word: &str) -> bool { + word == "…" || word == "..." +} + +/// Alternatives a single word stands for: `enable|disable`, or +/// `activate/rollback` in a subcommand position. Ellipsis alternatives +/// (`openai|...`) are dropped. +fn word_alternatives(word: &str, in_subcommand_position: bool) -> Vec { + let alternatives: Vec = if word.contains('|') && !word.starts_with('<') { + word.split('|').map(str::to_owned).collect() + } else if in_subcommand_position + && word.contains('/') + && word + .split('/') + .all(|part| !part.is_empty() && part.chars().all(|c| c.is_ascii_lowercase())) + { + // Model ids (`Qwen/Qwen3`) carry uppercase and never sit here. + word.split('/').map(str::to_owned).collect() + } else { + vec![word.to_owned()] + }; + alternatives + .into_iter() + .filter(|alt| !alt.is_empty() && !is_ellipsis(alt)) + .collect() +} + +/// What `rocm --yes …` stands for: the request form, spelled out the way the +/// docs do, so it reads as an advised natural-language request. +const NATURAL_LANGUAGE_REQUEST: &str = ""; + +/// A synopsis item: a required word (with alternatives) or an optional +/// `[...]` group (with `|`-separated alternative word lists). +enum Item { + Required(Vec), + Optional(Vec>), +} + +fn synopsis_items(words: &[String]) -> Vec { + let mut items = Vec::new(); + let mut group: Option>> = None; + for (index, raw_word) in words.iter().enumerate() { + let opens = raw_word.starts_with('['); + let closes = raw_word.ends_with(']'); + let word = raw_word.trim_matches(['[', ']']); + if opens && group.is_none() { + group = Some(vec![Vec::new()]); + } + if let Some(alternatives) = group.as_mut() { + if word == "|" { + alternatives.push(Vec::new()); + } else if !word.is_empty() && !is_ellipsis(word) { + // `[--provider anthropic|openai|...]`: a value list inside a + // group becomes one alternative per value. + let values = word_alternatives(word, false); + let current = alternatives.pop().unwrap_or_default(); + if values.len() > 1 && !current.is_empty() { + for value in values { + let mut alternative = current.clone(); + alternative.push(value); + alternatives.push(alternative); + } + } else { + let mut current = current; + current.extend(values); + alternatives.push(current); + } + } + if closes { + let alternatives = group.take().expect("open group"); + items.push(Item::Optional( + alternatives.into_iter().filter(|a| !a.is_empty()).collect(), + )); + } + continue; + } + if is_ellipsis(word) { + // `rocm --yes ...`: the ellipsis stands for the request. + if index > 0 && words[index - 1] == "--yes" { + items.push(Item::Required(vec![NATURAL_LANGUAGE_REQUEST.to_owned()])); + } + continue; + } + let in_subcommand_position = items.len() <= 1; + let alternatives = word_alternatives(word, in_subcommand_position); + if !alternatives.is_empty() { + items.push(Item::Required(alternatives)); + } + } + items +} + +/// One argv word, and the advice word it was filled in from: the same text, +/// or a placeholder (``) that [`placeholder_value`] substituted. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Word { + pub value: String, + pub advised: String, +} + +/// The argv variants one command (an item of [`command_parts`]) stands for. +/// Synopsis notation is expanded: each `[...]` optional group is tried on its +/// own (groups may be mutually exclusive, as in `rocm dash [--demo] [--replay +/// ]`), each `a|b` alternative produces a variant, `…`/`...` are +/// dropped, and placeholders are substituted. +fn command_variants(command: &str) -> Vec> { + let mut words = split_words(command); + if words.is_empty() { + return Vec::new(); + } + let program = words.remove(0); + let items = synopsis_items(&words); + + // `None` = required words only; `Some((group, alternative))` adds one + // optional group's alternative. + let mut selections: Vec> = vec![None]; + let mut group_count = 0; + for item in &items { + if let Item::Optional(alternatives) = item { + for alternative in 0..alternatives.len() { + selections.push(Some((group_count, alternative))); + } + group_count += 1; + } + } + + let start = Word { + value: program.clone(), + advised: program, + }; + let mut variants: Vec> = Vec::new(); + for selection in selections { + let mut partial: Vec> = vec![vec![start.clone()]]; + let mut group_index = 0; + for item in &items { + let choices: Vec> = match item { + Item::Required(alternatives) => { + alternatives.iter().map(|alt| vec![alt.clone()]).collect() + } + Item::Optional(alternatives) => { + let this = group_index; + group_index += 1; + match selection { + Some((group, alternative)) if group == this => { + vec![alternatives[alternative].clone()] + } + _ => continue, + } + } + }; + let mut next = Vec::new(); + for variant in &partial { + for choice in &choices { + let mut extended = variant.clone(); + for word in choice { + if is_options_placeholder(word) { + continue; + } + let previous = extended.last().map_or("", |w| w.value.as_str()); + let value = substitute_placeholders(word, previous); + extended.push(Word { + value, + advised: word.clone(), + }); + } + next.push(extended); + } + } + partial = next; + } + for variant in partial { + if !variants.contains(&variant) { + variants.push(variant); + } + } + } + variants +} + +fn values(words: &[Word]) -> Vec { + words.iter().map(|word| word.value.clone()).collect() +} + +/// The argv variants an advised string stands for: those of every command it +/// names (see [`command_parts`] and [`command_variants`]). +pub(crate) fn argv_variants(raw: &str) -> Vec> { + command_parts(raw) + .iter() + .flat_map(|command| command_variants(command)) + .map(|words| values(&words)) + .collect() +} + +// --------------------------------------------------------------------------- +// Verdict: the routing `run()` applies +// --------------------------------------------------------------------------- + +fn is_empty_value_error(error: &clap::Error) -> bool { + error.kind() == ErrorKind::InvalidValue + && matches!( + error.get(ContextKind::InvalidValue), + Some(ContextValue::String(value)) if value.is_empty() + ) +} + +fn classify_clap_error(error: &clap::Error) -> Verdict { + match error.kind() { + ErrorKind::DisplayHelp | ErrorKind::DisplayVersion => Verdict::Parses, + ErrorKind::MissingRequiredArgument + | ErrorKind::MissingSubcommand + | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => Verdict::IncompleteReference, + _ if is_empty_value_error(error) => Verdict::IncompleteReference, + _ => Verdict::Rejected(error.to_string()), + } +} + +/// Route `rocm ` exactly as `run()` does: natural-language requests go +/// to the planner (unless they look like a mistyped command), everything else +/// through `cli_command()` and `Cli`. +pub(crate) fn rocm_verdict(args: &[String]) -> Verdict { + if args.is_empty() { + return Verdict::Parses; // bare `rocm` opens the launcher. + } + let invocation = parse_freeform_invocation(args); + if should_treat_as_freeform(&invocation) { + return match command_invocation_error(&invocation.request_args) { + Some(error) => Verdict::Rejected(error.to_string()), + None => Verdict::Freeform, + }; + } + let argv = std::iter::once("rocm".to_owned()).chain(args.iter().cloned()); + let parsed = cli_command() + .try_get_matches_from(argv) + .and_then(|matches| Cli::from_arg_matches(&matches).map(|_| ())); + match parsed { + Ok(()) => Verdict::Parses, + Err(error) => classify_clap_error(&error), + } +} + +/// `rocmd`'s parser is private to its crate. Appending `--help` makes clap stop +/// at the first word it cannot place (unknown subcommand or flag, invalid enum +/// or number) and otherwise return `DisplayHelp` *before anything runs*, so +/// every named word is checked against the real definition without executing +/// it. Missing required values are not checked this way; for advice that is +/// the `IncompleteReference` case anyway. +pub(crate) fn rocmd_verdict(args: &[String]) -> Verdict { + let argv = std::iter::once("rocmd".into()) + .chain(args.iter().map(Into::into)) + .chain(std::iter::once("--help".into())) + .collect(); + match rocmd::run_from_args(argv) { + Ok(()) => Verdict::Parses, + Err(error) => match error.downcast_ref::() { + Some(clap_error) => classify_clap_error(clap_error), + None => Verdict::Rejected(format!("{error:#}")), + }, + } +} + +pub(crate) fn verdict(argv: &[String]) -> Verdict { + match argv.split_first() { + Some((program, args)) if program == "rocm" => rocm_verdict(args), + Some((program, args)) if program == "rocmd" => rocmd_verdict(args), + _ => Verdict::Rejected("not a rocm/rocmd invocation".to_owned()), + } +} + +// --------------------------------------------------------------------------- +// Exclusions and the contract +// --------------------------------------------------------------------------- + +/// Extracted strings that start with `rocm ` / `rocmd ` but are not advice to +/// run anything, keyed by `(file, exact extracted text)`. Each carries the +/// reason. Keep it short: an entry is something the contract does not cover, +/// and an entry whose text no longer occurs fails +/// `exclusions_still_match_something`. +const NOT_INVOCATIONS: &[(&str, &str, &str)] = &[ + // Prose that happens to begin with the program name. + ( + "crates/rocm-core/src/diagnose.rs", + "rocm serve/rocm chat would see this result, a plain shell might not", + "note naming the two commands a managed runtime affects, not advice", + ), + ( + "crates/rocm-core/src/diagnose.rs", + "rocm serve/rocm chat prepend the active managed runtime's own directories ahead of LD_LIBRARY_PATH, so the export below may not change what they load; reinstalling or repairing that runtime so it ships its own code object manager is the fix that reaches it directly.", + "explanation of what the two commands load, not advice", + ), + ( + "apps/rocm/src/dash.rs", + "rocm bench load supports http:// endpoints only (no TLS backend compiled in)", + "error message naming the command, not advice", + ), + ( + "apps/rocm/src/main.rs", + "rocm interactive shell", + "heading of the non-interactive launcher report", + ), + ( + "apps/rocm/src/main.rs", + "rocm tools: {}", + "`rocm tools: enabled` status row", + ), + ( + "crates/rocm-core/src/model_readiness.rs", + "rocm diagnose --model {}: {}", + "heading of the model-readiness report, `: `, naming what was run", + ), + ( + "apps/rocm/src/main.rs", + "rocm install folder", + "natural-language phrase the planner matches in a request", + ), + ( + "apps/rocm/src/main.rs", + "rocm installed at", + "natural-language phrase the planner matches in a request", + ), + ( + "apps/rocm/src/main.rs", + "rocm installed", + "natural-language phrase the planner matches in a request", + ), + ( + "apps/rocm/src/main.rs", + "rocm please", + "natural-language phrase the planner matches in a request", + ), + ( + "apps/rocm/src/main.rs", + "rocm command requires at least one argument", + "validation error naming the chat `rocm` tool", + ), + ( + "apps/rocm/src/main.rs", + "rocm command", + "fallback label for an unrenderable chat tool call", + ), + ( + "apps/rocm/src/main.rs", + "rocm serve requires ROCm GPU execution; CPU mode is not a fallback path in rocm-cli", + "error message naming the command, not advice", + ), + ( + "apps/rocmd/src/lib.rs", + "rocmd executable has no parent directory", + "error message", + ), + ( + "apps/rocmd/src/lib.rs", + "rocmd automation supervisor started", + "log line", + ), + ( + "apps/rocmd/src/lib.rs", + "rocmd automation supervisor stopped", + "log line", + ), + ( + "crates/rocm-core/src/diagnose.rs", + "rocm diagnose covers Linux, Windows and WSL2. This host reports '{}', which the \ + catalog has no entries for, so nothing was checked -- this is not a clean bill of \ + health. Run `rocm examine --json` and report the platform upstream.", + "out-of-scope report prose", + ), + ( + "crates/rocm-core/src/diagnose.rs", + "rocm diagnose: out of scope for this platform.", + "report heading", + ), + ( + "crates/rocm-core/src/diagnose.rs", + "rocm diagnose: no known misconfiguration matched.", + "report heading", + ), + ( + "apps/rocm/src/main.rs", + "rocm config", + "heading of the `rocm config` report (`writeln!(output, \"rocm config\")`)", + ), + ( + "crates/rocm-dash-collectors/src/bench_load.rs", + "rocm bench load (local smoke)", + "`launcher` label recorded in a benchmark result, not advice", + ), + ( + "apps/rocm/src/main.rs", + "rocm services {} {service_id} --yes", + "the verb is a format argument; every value it takes is checked against the \ + real message by `service_action_retry_advice_parses_for_generated_ids`", + ), + ( + "crates/rocm-core/src/examine.rs", + "rocm examine supports Linux and Windows; got {}. This skill cannot help on this platform.", + "error message naming the command, not advice", + ), + ( + "crates/rocm-core/src/fix.rs", + "rocm fix can run it", + "remediation-flag wording (`rocm fix can run it`), not a command", + ), + ( + "crates/rocm-core/src/lib.rs", + "rocm debug: command capture {stage} failed for {}: {detail}", + "debug log line", + ), + ( + "crates/rocm-dash-tui/src/ui/command_screen.rs", + "rocm {}", + "echo of whatever the user typed into the command runner", + ), + // Deliberate negative examples: the doc asserts these are rejected. + ( + "docs/manual-testing.md", + "rocm services prune --any-age --older-than-hours 0", + "the doc's expected result is that the parser rejects this combination", + ), +]; + +/// Natural-language examples must reach the planner with a multi-word request: +/// that is what tells a deliberate example (`rocm "start a local model"`) apart +/// from a structured command that no longer exists. A single word routed to +/// the planner (`rocm doctor`) means the advised subcommand is not real — the +/// user gets a request plan instead of the command they were told about. +/// +/// The words must be multi-word *in the advice*: a quoted request, or a +/// placeholder that names one (``). A value +/// [`placeholder_value`] filled in does not count — `rocm frobnicate ` +/// fills `` with several words, but the advice names a subcommand. +fn is_deliberate_natural_language(words: &[Word]) -> bool { + let args = values(&words[1..]); + let request = parse_freeform_invocation(&args).request_args; + // The request is a suffix of the arguments (`--yes` is the only prefix). + let request_words = &words[words.len() - request.len()..]; + request_words + .iter() + .any(|word| word.advised.contains(char::is_whitespace)) +} + +pub(crate) struct Finding { + pub source: String, + pub raw: String, + pub argv: Vec, + pub reason: String, +} + +fn is_excluded(item: &Advice) -> bool { + NOT_INVOCATIONS + .iter() + .any(|(file, raw, _)| *raw == item.raw && item.source.starts_with(&format!("{file}:"))) +} + +/// Whether advice may name a command without its required values. Only inline +/// prose may ("pass `rocm serve --engine`"), or text that marks the omission +/// itself with an ellipsis (`rocm runtimes …`). A command line meant to be run +/// as written may not: if `rocm examine` grew a required argument, every bare +/// `rocm examine` in a RECIPE, a `next step:` line or a fenced example would +/// fail for the user who ran it. +fn may_omit_required_values(item: &Advice, command: &str) -> bool { + item.surface == Surface::InlineProse + || split_words(command) + .iter() + .any(|word| word.trim_matches(['[', ']']).ends_with('…') || word.ends_with("...")) +} + +pub(crate) fn findings(advice: &[Advice]) -> Vec { + let mut out = Vec::new(); + for item in advice { + if is_excluded(item) { + continue; + } + for (command, words) in command_parts(&item.raw).iter().flat_map(|command| { + command_variants(command) + .into_iter() + .map(move |words| (command, words)) + }) { + let argv = values(&words); + let reason = match verdict(&argv) { + Verdict::Parses => continue, + Verdict::IncompleteReference if may_omit_required_values(item, command) => continue, + Verdict::IncompleteReference => "a required argument or subcommand is missing: \ + this line is meant to be run as written" + .to_owned(), + Verdict::Freeform if is_deliberate_natural_language(&words) => continue, + Verdict::Freeform => "not a subcommand: `rocm` sends it to the natural-language \ + planner instead of running a command" + .to_owned(), + Verdict::Rejected(error) => error, + }; + out.push(Finding { + source: item.source.clone(), + raw: item.raw.clone(), + argv, + reason, + }); + } + } + out +} + +pub(crate) fn render_findings(findings: &[Finding]) -> String { + let mut report = String::new(); + for finding in findings { + let _ = writeln!( + report, + "- {}\n advised: `{}`\n argv: {:?}\n error: {}", + finding.source, + finding.raw, + finding.argv, + finding.reason.lines().next().unwrap_or_default() + ); + } + report +} + +fn all_advice() -> Vec { + let mut advice = source_advice(); + advice.extend(help_text_advice()); + advice +} + +/// Where an advised invocation was found, for the per-source floors. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Origin { + Help, + Rust, + Markdown, + Tape, +} + +fn origin(item: &Advice) -> Origin { + if item.source.starts_with('`') { + return Origin::Help; + } + let file = item.source.rsplit_once(':').map_or("", |(file, _)| file); + match Path::new(file).extension().and_then(|ext| ext.to_str()) { + Some("rs") => Origin::Rust, + Some("md") => Origin::Markdown, + Some("tape") => Origin::Tape, + _ => panic!("advice from an unknown source: {}", item.source), + } +} + +/// Guards the scanner itself, source by source: a broken extractor would make +/// `every_advised_command_parses` pass vacuously for everything it feeds. Each +/// source (help, Rust, Markdown, tapes) must still yield a named, long-lived +/// command and a floor of entries (set well below today's counts so ordinary +/// doc edits do not trip it). +#[test] +fn every_source_is_scanned() { + let advice = all_advice(); + let count = |wanted: Origin, surface: Surface| { + advice + .iter() + .filter(|item| origin(item) == wanted && item.surface == surface) + .count() + }; + let has = |source_prefix: &str, raw: &str, surface: Surface| { + advice.iter().any(|item| { + item.source.starts_with(source_prefix) + && item.raw.starts_with(raw) + && item.surface == surface + }) + }; + + // Help: the top-level EXAMPLES row and inline spans. + assert!( + has("`rocm --help`", "rocm examine", Surface::CommandLine), + "help EXAMPLES row `rocm examine` not found: help extraction is broken" + ); + assert!(count(Origin::Help, Surface::CommandLine) >= 20); + assert!(count(Origin::Help, Surface::InlineProse) >= 20); + // Rust: a dashboard `cmd` literal, labelled lines, and inline spans. + assert!( + has( + "crates/rocm-dash-tui/src/ui/tabs/rocm.rs:", + "rocm update", + Surface::CommandLine + ), + "dashboard `cmd: \"rocm update\"` not found: Rust literal extraction is broken" + ); + assert!( + has( + "apps/rocm/src/main.rs:", + "rocm services stop", + Surface::CommandLine + ), + "labelled `stop: rocm services stop …` line not found" + ); + assert!(count(Origin::Rust, Surface::CommandLine) >= 90); + assert!(count(Origin::Rust, Surface::InlineProse) >= 130); + // Markdown: the quick-start fenced line and an inline span. + assert!( + has("README.md:", "rocm install sdk", Surface::CommandLine), + "README fenced `rocm install sdk` not found: fenced-block extraction is broken" + ); + assert!( + has("README.md:", "rocm examine", Surface::InlineProse), + "README inline `rocm examine` not found: inline-span extraction is broken" + ); + assert!(count(Origin::Markdown, Surface::CommandLine) >= 100); + assert!(count(Origin::Markdown, Surface::InlineProse) >= 120); + assert!( + count(Origin::Markdown, Surface::CommandLine) + + count(Origin::Markdown, Surface::InlineProse) + >= 300 + ); + // Tapes: the CLI demo's first command. + assert!( + has("docs/tapes/cli.tape:", "rocm examine", Surface::CommandLine), + "`Type \"rocm examine\"` not found in the CLI tape: tape extraction is broken" + ); + assert!(count(Origin::Tape, Surface::CommandLine) >= 5); +} + +#[test] +fn every_advised_command_parses() { + let advice = all_advice(); + let found = findings(&advice); + assert!( + found.is_empty(), + "{} advised `rocm`/`rocmd` invocation(s) do not parse with the real CLI. \ + Fix the advice (or the CLI), or — only for text that is not advice to run \ + anything — add it to NOT_INVOCATIONS with a reason:\n{}", + found.len(), + render_findings(&found) + ); +} + +#[test] +fn exclusions_still_match_something() { + let advice = source_advice(); + for (file, raw, reason) in NOT_INVOCATIONS { + assert!( + advice + .iter() + .any(|item| item.raw == *raw && item.source.starts_with(&format!("{file}:"))), + "NOT_INVOCATIONS entry ({file}, {raw:?}: {reason}) no longer matches any \ + extracted text; remove it" + ); + } +} + +#[test] +fn checker_rejects_what_users_would_hit() { + // The verdicts the contract rests on, each against the real parser. + let argv = |text: &str| argv_variants(text).remove(0); + assert!(matches!( + verdict(&argv("rocm update --check")), + Verdict::Rejected(_) + )); + assert!(matches!( + verdict(&argv("rocm install --channel release")), + Verdict::Rejected(_) + )); + assert_eq!(verdict(&argv("rocm doctor")), Verdict::Freeform); + let words = |text: &str| command_variants(text).remove(0); + assert!(!is_deliberate_natural_language(&words("rocm doctor"))); + assert!(matches!( + verdict(&argv("rocm instal sdk")), + Verdict::Rejected(_) + )); + assert!(matches!( + verdict(&argv("rocmd run --no-such-flag")), + Verdict::Rejected(_) + )); + assert_eq!( + verdict(&argv("rocm install sdk --family ")), + Verdict::Parses + ); + assert_eq!( + verdict(&argv("rocm serve --engine")), + Verdict::IncompleteReference + ); + assert_eq!( + verdict(&argv("rocmd run --automations-enabled")), + Verdict::Parses + ); + assert!(is_deliberate_natural_language(&words( + "rocm --yes \"start a local model\"" + ))); +} + +#[test] +fn only_prose_or_an_ellipsis_may_leave_required_values_out() { + let advice = |raw: &str, surface| Advice { + source: "fixture.md:1".to_owned(), + raw: raw.to_owned(), + surface, + }; + // `runtimes activate` requires a runtime key. + assert!(findings(&[advice("rocm runtimes activate", Surface::InlineProse)]).is_empty()); + assert_eq!( + findings(&[advice("rocm runtimes activate", Surface::CommandLine)]).len(), + 1, + "a command line missing a required value is a violation" + ); + assert!(findings(&[advice("rocm runtimes …", Surface::CommandLine)]).is_empty()); + assert!(findings(&[advice("rocm serve --managed ...", Surface::CommandLine)]).is_empty()); +} + +#[test] +fn synopsis_notation_expands_to_each_documented_form() { + assert_eq!( + argv_variants("rocm dash [--demo] [--replay ]"), + vec![ + vec!["rocm", "dash"], + vec!["rocm", "dash", "--demo"], + vec!["rocm", "dash", "--replay", "/tmp/replay.json"], + ] + ); + assert_eq!( + argv_variants("rocm services stop|restart --yes"), + vec![ + vec!["rocm", "services", "stop", "svc-1", "--yes"], + vec!["rocm", "services", "restart", "svc-1", "--yes"], + ] + ); + assert_eq!( + argv_variants("rocm serve --engine vllm # then check"), + vec![vec!["rocm", "serve", "Qwen/Qwen3-0.6B", "--engine", "vllm"]] + ); + assert_eq!( + argv_variants("rocm --yes "), + vec![vec!["rocm", "--yes", "start a local model"]] + ); +} + +/// Templated advice, checked through the real message function rather than +/// its source text: `rocm services stop|restart ` without `--yes` fails +/// with `Try: rocm services --yes`. The id comes from +/// `generate_service_id`, which takes an arbitrary model reference, so the +/// model references cover every character class it handles differently +/// (alphanumerics kept, everything else — separators, whitespace, quotes, +/// shell metacharacters, non-ASCII, a leading `-` — mapped to `-`). An +/// exhaustive class corpus rather than random sampling: the function is a +/// per-character map, so one representative per class covers it. +#[test] +fn service_action_retry_advice_parses_for_generated_ids() { + use super::{AppPaths, SUPPORTED_ENGINES, run_approved_service_action}; + + // Never touched: the missing-`--yes` branch bails before any disk access. + let unused = std::env::temp_dir().join("rocm-advised-commands-never-created"); + let paths = AppPaths { + config_dir: unused.join("config"), + data_dir: unused.join("data"), + cache_dir: unused.join("cache"), + }; + let long = "x".repeat(80); + let model_refs = [ + "Qwen/Qwen3-0.6B", + "unsloth/Qwen3-0.6B-GGUF:Q4_0", + "C:\\models\\local.gguf", + " leading and trailing ", + "-starts-with-dash", + "--looks-like-a-flag", + "with \"quotes\" and 'apostrophes'", + "a;b|c&&d$(e)`f`", + "ünïcödé/模型", + "", + long.as_str(), + ]; + // The tools the `services stop` / `services restart` dispatch passes in. + let actions = [("stop_server", "stop"), ("restart_server", "restart")]; + for engine in SUPPORTED_ENGINES { + for model_ref in model_refs { + let id = rocm_core::generate_service_id(engine, model_ref); + for (tool, verb) in actions { + let error = run_approved_service_action(&paths, tool, &id, false) + .expect_err("an action without --yes is refused") + .to_string(); + let advised = error + .lines() + .find_map(|line| line.strip_prefix("Try: ")) + .unwrap_or_else(|| panic!("no `Try:` advice in: {error}")); + let argv = split_words(advised); + assert_eq!( + verdict(&argv), + Verdict::Parses, + "advice {advised:?} for id {id:?} does not parse" + ); + // It names the same action on the same service and carries the + // `--yes` the refusal asked for, so following it clears the gate. + assert_eq!(argv, ["rocm", "services", verb, id.as_str(), "--yes"]); + } + } + } +} + +#[test] +#[ignore = "report: prints every advised invocation and its verdict"] +fn dump_advised_invocations() { + let advice = all_advice(); + for item in &advice { + for argv in argv_variants(&item.raw) { + println!( + "{}\t{}\t{:?}\t{:?}\t{:?}", + item.source, + item.raw, + argv, + verdict(&argv), + item.surface + ); + } + } + for wanted in [Origin::Help, Origin::Rust, Origin::Markdown, Origin::Tape] { + for surface in [Surface::CommandLine, Surface::InlineProse] { + let count = advice + .iter() + .filter(|item| origin(item) == wanted && item.surface == surface) + .count(); + println!("COUNT\t{wanted:?}\t{surface:?}\t{count}"); + } + } + println!("TOTAL\t{}", advice.len()); +} + +#[cfg(test)] +fn fixture(raw: &str, surface: Surface) -> Advice { + Advice { + source: "fixture.md:1".to_owned(), + raw: raw.to_owned(), + surface, + } +} + +/// Every command in a shell list is advice, not only the first: a removed +/// subcommand after `&&`, `||`, `;` or `|` strands the user just the same. +#[test] +fn every_command_in_a_shell_list_is_checked() { + for raw in [ + "rocm update && rocm frobnicate --x", + "rocm update || rocm frobnicate --x", + "rocm update; rocm frobnicate --x", + "rocm update | rocm frobnicate --x", + "rocm update && rocmd frobnicate", + ] { + let found = findings(&[fixture(raw, Surface::CommandLine)]); + assert_eq!(found.len(), 1, "{raw}: {}", render_findings(&found)); + assert!( + found[0].argv[1] == "frobnicate", + "{raw}: {:?}", + found[0].argv + ); + } + // Commands for other tools in the list are not this contract's business. + for raw in [ + "rocm examine --json | jq .gpus", + "rocm update && echo done", + "cd /tmp; rocm examine", + ] { + assert!( + findings(&[fixture(raw, Surface::CommandLine)]).is_empty(), + "{raw}" + ); + } + // `|` inside a word, a quoted request, a placeholder or an optional group + // is notation, not a pipe. + assert_eq!( + command_parts("rocm services stop|restart --yes"), + vec!["rocm services stop|restart --yes"] + ); + assert_eq!( + command_parts("rocm \"start a model && check it | twice\""), + vec!["rocm \"start a model && check it | twice\""] + ); + assert_eq!( + command_parts("rocm serve [--x | --y] && rocm examine"), + vec!["rocm serve [--x | --y]", "rocm examine"] + ); +} + +/// A double space inside a command is just whitespace; only a help EXAMPLES +/// row puts a description column after one. +#[test] +fn a_double_space_does_not_hide_the_rest_of_a_command() { + let found = findings(&[fixture( + "rocm install sdk --channel release --bogus-flag", + Surface::CommandLine, + )]); + assert_eq!(found.len(), 1, "{}", render_findings(&found)); + assert_eq!( + examples_row(" rocm examine Check GPU, driver and runtime state"), + Some("rocm examine".to_owned()) + ); + assert_eq!( + examples_row(" rocm serve --engine vllm"), + Some("rocm serve --engine vllm".to_owned()) + ); + // An unindented line is not a table row: a string literal or help line + // that starts with a command keeps everything after a double space. + assert_eq!( + examples_row("rocm install sdk --channel release --bogus-flag"), + Some("rocm install sdk --channel release --bogus-flag".to_owned()) + ); + assert_eq!(examples_row("Usage: rocm [OPTIONS]"), None); +} + +#[test] +fn an_options_placeholder_stands_for_no_arguments() { + // `` names flags the reader picks from + // the advice above it; the command around it must still be checked. + assert_eq!( + argv_variants("rocm serve "), + vec![vec![ + "rocm".to_owned(), + "serve".to_owned(), + "Qwen/Qwen3-0.6B".to_owned(), + ]], + ); + // Only a placeholder that names options is dropped. + assert_eq!( + argv_variants("rocm serve "), + vec![vec![ + "rocm".to_owned(), + "serve".to_owned(), + "Qwen/Qwen3-0.6B".to_owned(), + ]], + ); +} + +/// A natural-language request is deliberate only when the advice itself +/// spells one out. A placeholder filled with a multi-word value does not make +/// `rocm frobnicate ` a request: `frobnicate` is a removed subcommand. +#[test] +fn only_advised_text_makes_a_request_deliberate() { + let found = findings(&[fixture("rocm frobnicate ", Surface::CommandLine)]); + assert_eq!(found.len(), 1, "{}", render_findings(&found)); + for raw in [ + "rocm \"start a local model\"", + "rocm --yes \"start a local model\"", + "rocm --yes ", + "rocm --yes ...", + ] { + let found = findings(&[fixture(raw, Surface::CommandLine)]); + assert!(found.is_empty(), "{raw}: {}", render_findings(&found)); + } +} diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index d486ee8bc..4e1713d99 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -2,6 +2,8 @@ // // SPDX-License-Identifier: MIT +#[cfg(test)] +mod advised_commands; mod automations; mod bootstrap; mod chat_host_facts; diff --git a/crates/rocm-dash-tui/src/ui/examine_manager.rs b/crates/rocm-dash-tui/src/ui/examine_manager.rs index e34a5f90a..25c350eff 100644 --- a/crates/rocm-dash-tui/src/ui/examine_manager.rs +++ b/crates/rocm-dash-tui/src/ui/examine_manager.rs @@ -81,6 +81,9 @@ pub fn open_running(jobs: &mut State) -> (ExamineManagerState, Vec) (d, fx) } +/// The `rocm` argv (after the binary) this screen runs. +pub(crate) const EXAMINE_ARGS: &[&str] = &["examine"]; + /// Spawn `rocm examine` (read-only). A stable id replaces any prior console. fn run_examine(d: &mut ExamineManagerState, jobs: &mut State) -> Vec { let cmd = resolve_exe(); @@ -88,7 +91,7 @@ fn run_examine(d: &mut ExamineManagerState, jobs: &mut State) -> Vec let fx = jobs.apply(StateEvent::StartJob { id: id.clone(), cmd, - args: vec!["examine".to_string()], + args: EXAMINE_ARGS.iter().map(|arg| (*arg).to_string()).collect(), }); // Examine uses a single stable id, so a no-op (a prior run still going) // means re-attach to that same console — intentional, unlike the diff --git a/crates/rocm-dash-tui/src/ui/install_manager.rs b/crates/rocm-dash-tui/src/ui/install_manager.rs index 8ad5bb1aa..33fef2edb 100644 --- a/crates/rocm-dash-tui/src/ui/install_manager.rs +++ b/crates/rocm-dash-tui/src/ui/install_manager.rs @@ -133,7 +133,7 @@ impl InstallManagerState { } /// Build the `rocm install sdk …` argv, or an error message. - fn build_args(&self) -> Result, String> { + pub(crate) fn build_args(&self) -> Result, String> { let channel = self.channel.trim(); if channel.is_empty() { return Err("channel is required (e.g. release)".to_string()); diff --git a/crates/rocm-dash-tui/src/ui/tabs/rocm.rs b/crates/rocm-dash-tui/src/ui/tabs/rocm.rs index 1d00cfbe1..f219f1d32 100644 --- a/crates/rocm-dash-tui/src/ui/tabs/rocm.rs +++ b/crates/rocm-dash-tui/src/ui/tabs/rocm.rs @@ -30,7 +30,7 @@ pub const VERBS: &[Verb] = &[ "Pick an install folder (prefix)", "Dry-run to preview, then apply", ], - cmd: "rocm install --channel … --format …", + cmd: "rocm install sdk --channel … --format …", read_only: false, badge: None, }, @@ -44,7 +44,7 @@ pub const VERBS: &[Verb] = &[ "Preview the update (dry-run)", "Apply the update and activate it", ], - cmd: "rocm update --check", + cmd: "rocm update", read_only: false, badge: None, }, @@ -54,11 +54,11 @@ pub const VERBS: &[Verb] = &[ action: KeyAction::OpenExamine, summary: "Read-only environment check that flags what needs fixing.", steps: &[ - "Run `rocm doctor` (one job, no approval)", + "Run `rocm examine` (one job, no approval)", "Review runtime / driver / permission checks", "Re-run after fixes with r", ], - cmd: "rocm doctor", + cmd: "rocm examine", read_only: true, badge: None, }, @@ -187,6 +187,61 @@ mod tests { ); } + fn runs_label(action: KeyAction) -> &'static str { + VERBS + .iter() + .find(|verb| verb.action == action) + .unwrap_or_else(|| panic!("no ROCm verb opens {action:?}")) + .cmd + } + + /// The detail pane prints `Runs: `. A label that parses is not a + /// label that is true: each must name what its manager actually spawns, + /// so a user who copies it to a shell gets the same command. + #[test] + fn rocm_runs_labels_name_what_each_manager_spawns() { + use crate::ui::examine_manager::EXAMINE_ARGS; + use crate::ui::install_manager::InstallManagerState; + use crate::ui::update_manager::UpdateAction; + + assert_eq!( + runs_label(KeyAction::OpenUpdate), + format!("rocm {}", UpdateAction::Check.args().join(" ")), + "the Update row's label must be the command its first action runs" + ); + assert_eq!( + runs_label(KeyAction::OpenExamine), + format!("rocm {}", EXAMINE_ARGS.join(" ")), + "the doctor row's label must be the command the examine screen runs" + ); + + // Install takes user-chosen values, so its label shows `…` for them. + // Derive the label from the default form's argv: the subcommand, then + // each value flag with its value elided. Mode switches (`--dry-run`, + // or the approval flag a real install adds) depend on what the user + // picks in the form, so the label leaves them out. + let install = runs_label(KeyAction::OpenInstall); + let args = InstallManagerState::default() + .build_args() + .expect("the default install form builds an argv"); + let mut expected = vec!["rocm".to_owned()]; + let mut words = args.iter().peekable(); + while let Some(word) = words.next() { + if !word.starts_with("--") { + expected.push(word.clone()); + } else if words.peek().is_some_and(|next| !next.starts_with("--")) { + words.next(); + expected.push(word.clone()); + expected.push("…".to_owned()); + } + } + assert_eq!( + install, + expected.join(" "), + "the Install row's label must name what the form runs, values elided" + ); + } + #[test] fn rocm_verb_action_maps_selection_to_seam() { assert_eq!(verb_action(0), KeyAction::OpenInstall); diff --git a/crates/rocm-dash-tui/src/ui/update_manager.rs b/crates/rocm-dash-tui/src/ui/update_manager.rs index 4061fb166..1ba48d01b 100644 --- a/crates/rocm-dash-tui/src/ui/update_manager.rs +++ b/crates/rocm-dash-tui/src/ui/update_manager.rs @@ -58,7 +58,7 @@ impl UpdateAction { } /// `rocm` argv (after the binary) for this action. - fn args(self) -> Vec { + pub(crate) fn args(self) -> Vec { match self { Self::Check => vec!["update".into()], Self::Preview => vec!["update".into(), "--apply".into(), "--dry-run".into()], diff --git a/docs/testing.md b/docs/testing.md index e66614e2a..3c340d0ec 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -113,6 +113,78 @@ all of which the scan reports nothing for: `cfg_attr`. Write `#[cfg(unix)]` above `#[test]` instead, which the scan arms on. +### Advised commands must parse + +`apps/rocm/src/advised_commands.rs` checks the `rocm`/`rocmd` commands that the +CLI and its docs tell a user to run against the real parser. It is a text +scanner: it finds commands by a leading `rocm ` or `rocmd ` word in these +places, and checks only what it finds: + +- the `--help` of every visible command: lines that start with a command + (EXAMPLES rows) and inline backtick spans +- production Rust string literals: backtick spans, literals that start with a + command, and labelled lines such as `next step: rocm …`. Comments and + `#[cfg(test)]` items are skipped. +- `README.md`, `docs/` and `skills/`: inline backtick spans and fenced + code-block lines +- the VHS tapes under `docs/tapes/` + +Each line is split into the commands of a shell list (at `&&`, `||`, `;` and a +`|` standing between spaces, but not inside quotes, `<…>`, `[…]` or `{…}`), and +every command that runs `rocm` or `rocmd` is checked; commands for other tools +are skipped. A command ends at a shell comment, a redirection, or where +trailing prose starts (` (`, ` —`, ` → `, ` then `). A double space does not +end it, except on an indented EXAMPLES row, where the description column +follows one. + +It fills in placeholders such as `` and `{}`, drops one that stands for +options described elsewhere (``), tries each +`[--flag]` group and `a|b` alternative, then routes each command the way `rocm` +itself does. +These fail the test: + +- a command that clap rejects +- a command that ends up at the natural-language planner, unless the advice + itself is a multi-word request: a quoted one (`rocm "start a local model"`) + or a placeholder that names one (`rocm --yes `). A + placeholder filled in with several words does not count, so a removed + subcommand that took a text argument (`frobnicate `) still fails. +- a command that leaves out a required argument or subcommand, when it is a + line meant to be run as written: a fenced code line, a string literal that + starts with a command, a labelled `next step:`/`Try:` line, or a help + EXAMPLES row + +Only an inline backtick span in prose (``pass `rocm serve --engine` ``) may name +a command without its values, or a command that marks the gap with `…`. + +`every_source_is_scanned` requires each of the four sources to still yield a +named command (the `rocm examine` EXAMPLES row, the dashboard's `rocm update` +literal, README's fenced `rocm install sdk` and inline `rocm examine`, the CLI +tape's `rocm examine`) and a minimum count per source and surface, so a broken +extractor cannot pass silently. + +Known gaps of the text scanner. Advice written these ways is not checked, or +not checked as written: + +- a Rust raw string (`r#"…"#`) or a string literal that spans lines without a + trailing `\` is not read as one literal +- a backtick span that wraps across lines in a Rust string is not seen + (Markdown joins wrapped spans; Rust does not) +- a `{` or `}` inside a string in a `#[cfg(test)]` item can end the skipped + item early or late +- templated advice whose verb is a format argument (the `services` retry line + in `main.rs`) is excluded and checked through the function that builds it + instead + +When it fails, fix the advice (or the CLI). Add text to `NOT_INVOCATIONS`, with +a reason, only when the text starts with `rocm` but is not advice to run +anything, such as a log line or an error message that names the command. To see +every command it found and the result for each: + +```bash +cargo test -p rocm --bin rocm advised_commands::dump_advised_invocations -- --ignored --nocapture +``` + Run the cross-platform smoke test: ```bash