From b9301bbb098e7d231ee091cff63f4d98ccc7ff0c Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 4 Aug 2026 17:14:57 -0700 Subject: [PATCH 1/2] fix(output): prefix pagination next_actions with the binary name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suggested next-page command in `next_actions` started at the noun/verb path (e.g. `list --limit 2 --offset 2`), which isn't directly runnable — a user has to know to prepend their own binary name before pasting it. `pagination_command_base` now takes the CLI's configured binary name and leads with it, so the suggestion is copy-pastable as-is (e.g. `my-cli list --limit 2 --offset 2`). Co-Authored-By: Claude Sonnet 5 --- cli-engine/src/cli.rs | 26 +++++++++++++++++--------- cli-engine/tests/pagination.rs | 20 ++++++++++---------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/cli-engine/src/cli.rs b/cli-engine/src/cli.rs index 773d599..690984f 100644 --- a/cli-engine/src/cli.rs +++ b/cli-engine/src/cli.rs @@ -1864,11 +1864,15 @@ impl Cli { apply_pagination_flags(&mut middleware, &command.spec, leaf); let args = command_args_from_matches(leaf, &command.spec, false); let user_args = command_args_from_matches(leaf, &command.spec, true); - let pagination_command = command - .spec - .pagination - .is_some() - .then(|| pagination_command_base(&command_path, &command.spec, &user_args, &flags)); + let pagination_command = command.spec.pagination.is_some().then(|| { + pagination_command_base( + &self.config.name, + &command_path, + &command.spec, + &user_args, + &flags, + ) + }); if let Err(err) = self.run_pre_run(&mut middleware, &command_path, &args) { return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id)); } @@ -2571,9 +2575,12 @@ fn apply_pagination_flags(middleware: &mut Middleware, spec: &CommandSpec, leaf: } /// Replays a paginating command's own explicit args, plus the global -/// `--filter`/`--expr`/`--fields` flags, as `--flag value` text — the base a -/// "view the next page" [`crate::NextAction`] is built from once the -/// response's [`crate::PaginationMeta`] is known. +/// `--filter`/`--expr`/`--fields` flags, as `--flag value` text, prefixed +/// with the CLI's binary name — the base a "view the next page" +/// [`crate::NextAction`] is built from once the response's +/// [`crate::PaginationMeta`] is known. Leading with the binary name keeps the +/// suggested command copy-pastable rather than a fragment starting at the +/// noun/verb path. /// /// `--filter`/`--expr`/`--fields` sit in the same output pipeline as /// pagination itself (filter -> paginate -> expr -> fields) and change what @@ -2594,12 +2601,13 @@ fn apply_pagination_flags(middleware: &mut Middleware, spec: &CommandSpec, leaf: /// those are added by the caller once it knows the /// next page's offset. fn pagination_command_base( + binary_name: &str, command_path: &str, spec: &CommandSpec, user_args: &crate::middleware::ValueMap, flags: &GlobalFlags, ) -> String { - let mut parts = vec![command_path.replace(':', " ")]; + let mut parts = vec![binary_name.to_owned(), command_path.replace(':', " ")]; for arg in &spec.args { let id = arg.get_id().as_str(); if let Some(value) = user_args.get(id) { diff --git a/cli-engine/tests/pagination.rs b/cli-engine/tests/pagination.rs index 5727e68..5439740 100644 --- a/cli-engine/tests/pagination.rs +++ b/cli-engine/tests/pagination.rs @@ -98,7 +98,7 @@ async fn default_limit_applies_when_neither_flag_is_passed() { ); assert_eq!( rendered["next_actions"][0]["command"], - "list --limit 2 --offset 2" + "my-cli list --limit 2 --offset 2" ); assert!(rendered.get("metadata").is_none(), "{}", output.rendered); } @@ -131,7 +131,7 @@ async fn explicit_limit_and_offset_override_the_default_and_expose_pagination() ); assert_eq!( rendered["next_actions"][0]["command"], - "list --limit 2 --offset 3" + "my-cli list --limit 2 --offset 3" ); assert_eq!( rendered["next_actions"][0]["description"], @@ -184,7 +184,7 @@ async fn next_page_action_replays_other_flags_the_user_passed() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --status active --limit 2 --offset 2" + "my-cli list --status active --limit 2 --offset 2" ); } @@ -214,7 +214,7 @@ async fn next_page_action_quotes_values_with_whitespace() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --status \"in review\" --limit 2 --offset 2" + "my-cli list --status \"in review\" --limit 2 --offset 2" ); } @@ -250,7 +250,7 @@ async fn next_page_action_uses_the_real_long_flag_not_the_value_map_key() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --sort-order asc --limit 2 --offset 2" + "my-cli list --sort-order asc --limit 2 --offset 2" ); } @@ -365,7 +365,7 @@ async fn human_output_shows_pagination_summary_and_next_steps() { output.rendered ); assert!( - output.rendered.contains("list --limit 2 --offset 2"), + output.rendered.contains("my-cli list --limit 2 --offset 2"), "{}", output.rendered ); @@ -433,7 +433,7 @@ async fn next_page_action_preserves_filter_expr_and_fields() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --filter \"name != 'alpha'\" --expr \"sort_by(@, &name)\" --fields name --limit 2 --offset 2" + "my-cli list --filter \"name != 'alpha'\" --expr \"sort_by(@, &name)\" --fields name --limit 2 --offset 2" ); } @@ -463,7 +463,7 @@ async fn next_page_action_replays_a_set_false_flag_as_a_bare_switch() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --no-cache --limit 2 --offset 2" + "my-cli list --no-cache --limit 2 --offset 2" ); } @@ -531,7 +531,7 @@ async fn next_page_action_replays_a_multi_value_arg_as_repeated_flags() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - "list --scope a --scope b --limit 2 --offset 2" + "my-cli list --scope a --scope b --limit 2 --offset 2" ); } @@ -607,7 +607,7 @@ async fn next_page_action_escapes_shell_metacharacters_and_expansions() { let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); assert_eq!( rendered["next_actions"][0]["command"], - r#"list --status "a;\$(whoami)\`x\`\"y\"\\z" --limit 2 --offset 2"# + r#"my-cli list --status "a;\$(whoami)\`x\`\"y\"\\z" --limit 2 --offset 2"# ); } From 2f4adebc30eb833c4e0cb044348ddf8748444486 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 4 Aug 2026 17:20:48 -0700 Subject: [PATCH 2/2] fix(output): quote the binary name in pagination next_actions Copilot review: pagination_command_base prefixed the suggested next-page command with CliConfig::name unquoted, while every other token in the suggestion goes through quote_pagination_value. A binary name containing whitespace or shell metacharacters (CliConfig::new doesn't validate name) would produce a suggestion that isn't actually copy-pastable, defeating the point of this fix. Route the binary name through the same quoting helper and add a regression test. Co-Authored-By: Claude Sonnet 5 --- cli-engine/src/cli.rs | 5 ++++- cli-engine/tests/pagination.rs | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/cli-engine/src/cli.rs b/cli-engine/src/cli.rs index 690984f..b058464 100644 --- a/cli-engine/src/cli.rs +++ b/cli-engine/src/cli.rs @@ -2607,7 +2607,10 @@ fn pagination_command_base( user_args: &crate::middleware::ValueMap, flags: &GlobalFlags, ) -> String { - let mut parts = vec![binary_name.to_owned(), command_path.replace(':', " ")]; + let mut parts = vec![ + quote_pagination_value(binary_name), + command_path.replace(':', " "), + ]; for arg in &spec.args { let id = arg.get_id().as_str(); if let Some(value) = user_args.get(id) { diff --git a/cli-engine/tests/pagination.rs b/cli-engine/tests/pagination.rs index 5439740..c65ebb1 100644 --- a/cli-engine/tests/pagination.rs +++ b/cli-engine/tests/pagination.rs @@ -218,6 +218,28 @@ async fn next_page_action_quotes_values_with_whitespace() { ); } +#[tokio::test] +async fn next_page_action_quotes_a_binary_name_with_whitespace() { + let mut cli = Cli::new(CliConfig::new("my cli", "Dev tooling", "my-cli")); + cli.add_command(RuntimeCommandSpec::new( + CommandSpec::new("list", "List things") + .no_auth(true) + .with_pagination(PaginationConfig { + default_limit: 2, + ..PaginationConfig::default() + }), + async |_credential, _args| Ok(CommandResult::new(json!(items()))), + )); + + let output = cli.run(["my cli", "list", "--output", "json"]).await; + assert_eq!(output.exit_code, 0, "{}", output.rendered); + let rendered: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json"); + assert_eq!( + rendered["next_actions"][0]["command"], + "\"my cli\" list --limit 2 --offset 2" + ); +} + #[derive(Debug, Clone, clap::Args)] struct ListArgs { #[arg(long)]