From 79bd20fe07ee484f5f67468f372939792fa4b170 Mon Sep 17 00:00:00 2001 From: Ulysse Mavrocordatos Date: Fri, 18 Sep 2026 12:27:55 +0200 Subject: [PATCH 1/3] fix(cli): sort top-level help by display order Use a shared clap command constructor so generated and hand-written commands are alphabetized together in help and completions. Retarget the regression test at rendered help and cover an appended out-of-order command. --- src/commands/completions.rs | 5 ++--- src/main.rs | 6 +++++- src/test_commands.rs | 42 ++++++++++++++++++++++++++++++------- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/commands/completions.rs b/src/commands/completions.rs index 64672096..c1f36816 100644 --- a/src/commands/completions.rs +++ b/src/commands/completions.rs @@ -1,14 +1,13 @@ use anyhow::{Context, Result}; -use clap::CommandFactory; use clap_complete::Shell; use std::fs; use std::path::PathBuf; -use crate::Cli; +use crate::cli_command; /// Generate completions to stdout (the raw clap output). pub fn generate(shell: Shell) { - clap_complete::generate(shell, &mut Cli::command(), "pup", &mut std::io::stdout()); + clap_complete::generate(shell, &mut cli_command(), "pup", &mut std::io::stdout()); } /// Install a dynamic loader script for the given shell. diff --git a/src/main.rs b/src/main.rs index a036b17c..506b96b4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -81,6 +81,10 @@ pub(crate) struct Cli { command: Commands, } +pub(crate) fn cli_command() -> clap::Command { + Cli::command().mut_subcommands(|command| command.display_order(0)) +} + #[derive(Subcommand)] enum Commands { /// Start a local ACP server that proxies to Datadog Bits AI @@ -13384,7 +13388,7 @@ async fn main_inner() -> anyhow::Result<()> { // Build the clap Command and, when extensions are installed, append an // "EXTENSIONS:" section to the help output so they are visible in // `pup --help` / `pup help`, similar to how `gh` lists extensions. - let mut cmd = Cli::command(); + let mut cmd = cli_command(); #[cfg(not(target_arch = "wasm32"))] { let ext_help = extensions::discovery::build_extensions_help_section(); diff --git a/src/test_commands.rs b/src/test_commands.rs index 683dfcce..612290d4 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -463,17 +463,45 @@ fn test_extension_list_remote_parses() { #[test] fn test_top_level_commands_sorted_alphabetically() { - let app = crate::Cli::command(); - let names: Vec<&str> = app + let mut app = crate::cli_command(); + let visible_names: Vec = app .get_subcommands() .filter(|cmd| cmd.get_name() != "help" && !cmd.is_hide_set()) - .map(|cmd| cmd.get_name()) + .map(|cmd| cmd.get_name().to_string()) + .collect(); + let help = app.render_help().to_string(); + let names: Vec<&str> = help + .lines() + .skip_while(|line| line.trim() != "Commands:") + .skip(1) + .filter_map(|line| line.split_whitespace().next()) + .filter(|name| visible_names.iter().any(|visible| visible == name)) .collect(); - let mut sorted = names.clone(); - sorted.sort_unstable(); + let mut expected: Vec<&str> = visible_names.iter().map(String::as_str).collect(); + expected.sort_unstable(); + assert_eq!( + names, expected, + "top-level commands in help must be in alphabetical order.\nActual: {names:?}\nExpected: {expected:?}" + ); +} + +#[test] +fn test_shared_display_order_sorts_appended_subcommand_in_help() { + let mut app = + crate::cli_command().subcommand(clap::Command::new("downtime-z-test").display_order(0)); + let help = app.render_help().to_string(); + let names: Vec<&str> = help + .lines() + .skip_while(|line| line.trim() != "Commands:") + .skip(1) + .filter_map(|line| line.split_whitespace().next()) + .filter(|name| matches!(*name, "downtime" | "downtime-z-test" | "error-tracking")) + .collect(); + assert_eq!( - names, sorted, - "top-level commands must be in alphabetical order.\nActual: {names:?}\nExpected: {sorted:?}" + names, + ["downtime", "downtime-z-test", "error-tracking"], + "display order did not interleave the appended command:\n{help}" ); } From f2fc8bcaed3a7e518aac120dc992e088e7a86eed Mon Sep 17 00:00:00 2001 From: Ulysse Mavrocordatos Date: Fri, 18 Sep 2026 14:23:34 +0200 Subject: [PATCH 2/3] test(cli): harden help ordering invariants Use the shared command constructor for agent help, enforce unique top-level names and uniform display order, and make rendered-help parsing robust to wrapped descriptions and later help sections. --- src/main.rs | 4 ++- src/test_commands.rs | 76 ++++++++++++++++++++++++++++++++------------ 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/main.rs b/src/main.rs index 506b96b4..9fff5a90 100644 --- a/src/main.rs +++ b/src/main.rs @@ -81,6 +81,8 @@ pub(crate) struct Cli { command: Commands, } +/// Build the CLI with top-level subcommands sorted by name in help output. +/// Nested subcommands retain their declared display order. pub(crate) fn cli_command() -> clap::Command { Cli::command().mut_subcommands(|command| command.display_order(0)) } @@ -13337,7 +13339,7 @@ async fn main_inner() -> anyhow::Result<()> { let has_agent_flag = args.iter().any(|a| a == "--agent"); let has_no_agent_flag = args.iter().any(|a| a == "--no-agent"); if has_help && !has_no_agent_flag && (useragent::is_agent_mode() || has_agent_flag) { - let cmd = Cli::command(); + let cmd = cli_command(); if let Some(schema) = agent_help_schema(&cmd, &args) { println!("{}", serde_json::to_string_pretty(&schema).unwrap()); return Ok(()); diff --git a/src/test_commands.rs b/src/test_commands.rs index 612290d4..65f90cff 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -461,47 +461,83 @@ fn test_extension_list_remote_parses() { } } -#[test] -fn test_top_level_commands_sorted_alphabetically() { - let mut app = crate::cli_command(); - let visible_names: Vec = app - .get_subcommands() - .filter(|cmd| cmd.get_name() != "help" && !cmd.is_hide_set()) - .map(|cmd| cmd.get_name().to_string()) - .collect(); +fn visible_top_level_command_names(app: &clap::Command) -> Vec { + app.get_subcommands() + .filter(|command| command.get_name() != "help" && !command.is_hide_set()) + .map(|command| command.get_name().to_string()) + .collect() +} + +fn rendered_top_level_command_names(app: &mut clap::Command) -> Vec { + let visible_names = visible_top_level_command_names(app); let help = app.render_help().to_string(); - let names: Vec<&str> = help - .lines() + + help.lines() .skip_while(|line| line.trim() != "Commands:") .skip(1) + .take_while(|line| !line.trim().is_empty()) + .filter_map(|line| line.strip_prefix(" ")) + .filter(|line| !line.starts_with(' ')) .filter_map(|line| line.split_whitespace().next()) .filter(|name| visible_names.iter().any(|visible| visible == name)) - .collect(); - let mut expected: Vec<&str> = visible_names.iter().map(String::as_str).collect(); + .map(str::to_string) + .collect() +} + +#[test] +fn test_top_level_commands_sorted_alphabetically() { + let mut app = crate::cli_command(); + let mut expected = visible_top_level_command_names(&app); expected.sort_unstable(); + let names = rendered_top_level_command_names(&mut app); + assert_eq!( names, expected, "top-level commands in help must be in alphabetical order.\nActual: {names:?}\nExpected: {expected:?}" ); } +#[test] +fn test_top_level_commands_share_display_order() { + assert!( + crate::cli_command() + .get_subcommands() + .all(|command| command.get_display_order() == 0), + "top-level commands must share a display order so clap sorts them by name" + ); +} + +#[test] +fn test_top_level_command_names_are_unique() { + let mut names = std::collections::HashSet::new(); + + for command in crate::Cli::command().get_subcommands() { + assert!( + names.insert(command.get_name()), + "duplicate top-level command name: {}", + command.get_name() + ); + } +} + #[test] fn test_shared_display_order_sorts_appended_subcommand_in_help() { let mut app = crate::cli_command().subcommand(clap::Command::new("downtime-z-test").display_order(0)); - let help = app.render_help().to_string(); - let names: Vec<&str> = help - .lines() - .skip_while(|line| line.trim() != "Commands:") - .skip(1) - .filter_map(|line| line.split_whitespace().next()) - .filter(|name| matches!(*name, "downtime" | "downtime-z-test" | "error-tracking")) + let names: Vec = rendered_top_level_command_names(&mut app) + .into_iter() + .filter(|name| { + matches!( + name.as_str(), + "downtime" | "downtime-z-test" | "error-tracking" + ) + }) .collect(); assert_eq!( names, ["downtime", "downtime-z-test", "error-tracking"], - "display order did not interleave the appended command:\n{help}" + "display order did not interleave the appended command" ); } From f7df9fcbd4569891df90bed60706c2c66ef3e578 Mon Sep 17 00:00:00 2001 From: Ulysse Mavrocordatos Date: Fri, 18 Sep 2026 14:39:18 +0200 Subject: [PATCH 3/3] test(cli): include aliases in uniqueness invariant Prevent top-level command names and both visible and hidden aliases from colliding across hand-written and generated commands. --- src/test_commands.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/test_commands.rs b/src/test_commands.rs index 65f90cff..bbc6f2ff 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -508,15 +508,19 @@ fn test_top_level_commands_share_display_order() { } #[test] -fn test_top_level_command_names_are_unique() { - let mut names = std::collections::HashSet::new(); - - for command in crate::Cli::command().get_subcommands() { - assert!( - names.insert(command.get_name()), - "duplicate top-level command name: {}", - command.get_name() - ); +fn test_top_level_command_names_and_aliases_are_unique() { + let app = crate::Cli::command(); + let mut names = std::collections::HashMap::new(); + + for command in app.get_subcommands() { + for name in std::iter::once(command.get_name()).chain(command.get_all_aliases()) { + if let Some(existing) = names.insert(name, command.get_name()) { + panic!( + "top-level command name or alias `{name}` for `{}` conflicts with `{existing}`", + command.get_name() + ); + } + } } }