From 819352410ae3234c842d7a47b0f6bdbf4ec732d9 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 11:50:49 -0700 Subject: [PATCH 1/6] feat(auth): default human views and default fields for auth status/login The built-in `auth status` and `auth login` had no human view, so their columns rendered in alphabetical order and a consumer could not change them without replacing the whole auth group. - Render both through the views `auth:status` / `auth:login` (`AUTH_STATUS_VIEW_ID`, `AUTH_LOGIN_VIEW_ID`). The engine registers `default_auth_status_view()` / `default_auth_login_view()` (identity, expires, env, provider, ...) unless the consumer already registered a view under that id, so an override is never clobbered. - Add `CliConfig::with_auth_status_default_fields` and `with_auth_login_default_fields` to choose the default `--fields` without removing columns from the view. - Register a JSON schema for `AuthStatusEntry` / `AuthLoginResult` so `--help` lists every output field and `--schema` works. - Print `[default: ...]` for `--fields` on the flag's first help line instead of after the last row of the output-field table. - Only say `--interactive` is the default on a TTY when `auto_interactive` is enabled. Refs DEVEX-1267 Co-Authored-By: Claude Sonnet 5.5 --- cli-engine/docs/auth.md | 12 ++ cli-engine/src/auth/commands.rs | 64 +++++++- cli-engine/src/auth/mod.rs | 3 +- cli-engine/src/cli/config.rs | 24 +++ cli-engine/src/cli/mod.rs | 9 ++ cli-engine/src/cli/registration.rs | 33 +++- cli-engine/src/cli/schema_tree.rs | 9 +- cli-engine/src/flags/register.rs | 2 +- cli-engine/src/lib.rs | 5 +- cli-engine/tests/auth_views.rs | 249 +++++++++++++++++++++++++++++ cli-engine/tests/interactivity.rs | 23 +++ 11 files changed, 422 insertions(+), 11 deletions(-) create mode 100644 cli-engine/tests/auth_views.rs diff --git a/cli-engine/docs/auth.md b/cli-engine/docs/auth.md index d11a264..46ac71e 100644 --- a/cli-engine/docs/auth.md +++ b/cli-engine/docs/auth.md @@ -316,6 +316,18 @@ When a CLI registers auth providers or configures a default provider, `cli_engin These commands are implemented with the same `CommandSpec`, middleware, output envelope, and renderers as application commands. +### Human views + +`auth status` and `auth login` render their human output through the views `auth:status` and `auth:login` (`AUTH_STATUS_VIEW_ID`, `AUTH_LOGIN_VIEW_ID`). The engine registers `default_auth_status_view()` and `default_auth_login_view()` under those ids: identity first, then expiry, environment, provider, and the remaining fields. + +To change which columns show by default, set the command's default fields with `CliConfig::with_auth_status_default_fields` / `with_auth_login_default_fields`. + +```rust +let config = config.with_auth_status_default_fields("identity,expires_at,env,expired,refreshable,scopes"); +``` + +To change the columns themselves, register a `HumanViewDef` under the same id with `CliConfig::with_view` (or `register_global_human_view`). The built-in group is kept, and a consumer view always wins over the default. `default_auth_status_view().columns` is a convenient starting point. + ## Transport Injectors Transport injectors implement [`AuthInjector`](../src/transport/injector.rs) and mutate outbound diff --git a/cli-engine/src/auth/commands.rs b/cli-engine/src/auth/commands.rs index 33d3a8a..ae92255 100644 --- a/cli-engine/src/auth/commands.rs +++ b/cli-engine/src/auth/commands.rs @@ -1,13 +1,65 @@ use clap::{Arg, ArgAction}; +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use super::Dispatcher; use crate::{ - CliCoreError, CommandContext, CommandResult, CommandSpec, Credential, GroupSpec, Result, - RuntimeCommandSpec, RuntimeGroupSpec, Tier, + CliCoreError, CommandContext, CommandResult, CommandSpec, Credential, GroupSpec, HumanViewDef, + Result, RuntimeCommandSpec, RuntimeGroupSpec, TableColumn, Tier, }; +/// Human view id the built-in `auth status` command renders with. +/// +/// Register a [`HumanViewDef`] under this id (via +/// [`CliConfig::with_view`](crate::CliConfig::with_view) or +/// [`register_global_human_view`](crate::register_global_human_view)) to +/// replace the engine's [`default_auth_status_view`]. +pub const AUTH_STATUS_VIEW_ID: &str = "auth:status"; + +/// Human view id the built-in `auth login` command renders with. See +/// [`AUTH_STATUS_VIEW_ID`]. +pub const AUTH_LOGIN_VIEW_ID: &str = "auth:login"; + +/// The engine's default human view for `auth status`: identity and expiry +/// first, then the environment and provider they belong to. +/// +/// A consumer overriding [`AUTH_STATUS_VIEW_ID`] can start from this and +/// drop or reorder columns, for example to hide `provider` when only one is +/// registered. +#[must_use] +pub fn default_auth_status_view() -> HumanViewDef { + HumanViewDef::new( + AUTH_STATUS_VIEW_ID, + vec![ + TableColumn::new("identity", "Identity"), + TableColumn::new("expires_at", "Expires"), + TableColumn::new("env", "Env"), + TableColumn::new("provider", "Provider"), + TableColumn::new("expired", "Expired"), + TableColumn::new("refreshable", "Refreshable"), + TableColumn::new("scopes", "Scopes"), + ], + ) +} + +/// The engine's default human view for `auth login`. See +/// [`default_auth_status_view`]. +#[must_use] +pub fn default_auth_login_view() -> HumanViewDef { + HumanViewDef::new( + AUTH_LOGIN_VIEW_ID, + vec![ + TableColumn::new("identity", "Identity"), + TableColumn::new("expires_at", "Expires"), + TableColumn::new("env", "Env"), + TableColumn::new("provider", "Provider"), + TableColumn::new("refreshable", "Refreshable"), + TableColumn::new("scopes", "Scopes"), + ], + ) +} + /// Data rendered after a successful `auth login`. /// /// Built by [`login_and_build`]/[`login_and_build_with_scopes`] from the @@ -15,7 +67,7 @@ use crate::{ /// result and should not construct it directly — `#[non_exhaustive]` lets /// this framework add fields (as it did for [`refreshable`](Self::refreshable)) /// without another breaking release. -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] #[non_exhaustive] pub struct AuthLoginResult { /// Provider used for login. @@ -46,7 +98,7 @@ pub struct AuthLoginResult { /// result and should not construct it directly — `#[non_exhaustive]` lets /// this framework add fields (as it did for [`refreshable`](Self::refreshable)) /// without another breaking release. -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] #[non_exhaustive] pub struct AuthStatusEntry { /// Provider name. @@ -86,6 +138,8 @@ pub fn auth_command_group(default_provider: &str, registered_names: &[String]) - .with_tier(Tier::Mutate) .mutates(true) .no_auth(true) + .with_view_id(AUTH_LOGIN_VIEW_ID) + .with_json_schema::() .with_arg(provider_arg(&effective_default, registered_names)) .with_arg(Arg::new("env").long("env").value_name("ENV")) .with_arg( @@ -115,6 +169,8 @@ pub fn auth_command_group(default_provider: &str, registered_names: &[String]) - CommandSpec::new("status", "Show cached credential status") .with_system("auth") .no_auth(true) + .with_view_id(AUTH_STATUS_VIEW_ID) + .with_json_schema::() .with_arg(provider_arg(&effective_default, registered_names)) .with_arg(Arg::new("env").long("env").value_name("ENV")), async |context| { diff --git a/cli-engine/src/auth/mod.rs b/cli-engine/src/auth/mod.rs index 45fd6d6..ea87688 100644 --- a/cli-engine/src/auth/mod.rs +++ b/cli-engine/src/auth/mod.rs @@ -25,7 +25,8 @@ pub mod storage; use async_trait::async_trait; pub use commands::{ - AuthLoginResult, AuthStatusEntry, auth_command_group, login_and_build, + AUTH_LOGIN_VIEW_ID, AUTH_STATUS_VIEW_ID, AuthLoginResult, AuthStatusEntry, auth_command_group, + default_auth_login_view, default_auth_status_view, login_and_build, login_and_build_with_scopes, logout_result, status_result, to_status_entry, }; pub use credential::{CACHE_TTL, Credential}; diff --git a/cli-engine/src/cli/config.rs b/cli-engine/src/cli/config.rs index 28eebb2..92940ce 100644 --- a/cli-engine/src/cli/config.rs +++ b/cli-engine/src/cli/config.rs @@ -124,6 +124,12 @@ pub struct CliConfig { /// the built-in group is built, so the built-ins are never lost or /// overwritten. pub auth_extra_commands: Vec, + /// Default `--fields` for the built-in `auth status`. Populate via + /// [`CliConfig::with_auth_status_default_fields`]. + pub auth_status_default_fields: Option, + /// Default `--fields` for the built-in `auth login`. Populate via + /// [`CliConfig::with_auth_login_default_fields`]. + pub auth_login_default_fields: Option, /// Global guide entries mounted under `guide`. pub guides: Vec, /// Global human output views. @@ -497,6 +503,24 @@ impl CliConfig { self } + /// Sets the default `--fields` of the built-in `auth status`. + /// + /// Same semantics as [`CommandSpec::with_default_fields`](crate::CommandSpec::with_default_fields): the fields + /// shown (and their order) when `--fields` is absent. + #[must_use] + pub fn with_auth_status_default_fields(mut self, fields: impl Into) -> Self { + self.auth_status_default_fields = Some(fields.into()); + self + } + + /// Sets the default `--fields` of the built-in `auth login`. See + /// [`CliConfig::with_auth_status_default_fields`]. + #[must_use] + pub fn with_auth_login_default_fields(mut self, fields: impl Into) -> Self { + self.auth_login_default_fields = Some(fields.into()); + self + } + /// Adds one global guide. #[must_use] pub fn with_guide(mut self, guide: GuideEntry) -> Self { diff --git a/cli-engine/src/cli/mod.rs b/cli-engine/src/cli/mod.rs index b1707df..ef7212e 100644 --- a/cli-engine/src/cli/mod.rs +++ b/cli-engine/src/cli/mod.rs @@ -179,6 +179,15 @@ impl Cli { .subcommand(Command::new("tree").about("Display full command tree")) .subcommand(completion_command()) .subcommand(search_command()); + // The flag is only a default-on behavior for a CLI that opted into TTY + // auto-detection, so say so only then. + if config.auto_interactive { + root = root.mut_arg("interactive", |arg| { + arg.help( + "Force interactive prompts for missing inputs (default when a TTY is detected)", + ) + }); + } if let Some(register_flags) = &config.register_flags { root = register_flags(root); } diff --git a/cli-engine/src/cli/registration.rs b/cli-engine/src/cli/registration.rs index 9b93492..9c5cbc4 100644 --- a/cli-engine/src/cli/registration.rs +++ b/cli-engine/src/cli/registration.rs @@ -6,7 +6,7 @@ use clap::builder::PossibleValuesParser; use super::{ Cli, - config::{BUILTIN_COMMAND_NAMES, DEFAULT_ADMIN_CATEGORY}, + config::{BUILTIN_COMMAND_NAMES, CliConfig, DEFAULT_ADMIN_CATEGORY}, help::{ModuleHelpEntry, build_root_long}, lookup::has_subcommand, schema_tree::{ @@ -14,7 +14,11 @@ use super::{ runtime_group_clap_command_with_schema_help, }, }; -use crate::{FeatureFlag, RuntimeGroupSpec, auth::commands::auth_command_group}; +use crate::auth::commands::{ + auth_command_group, default_auth_login_view, default_auth_status_view, +}; +use crate::output::HumanViewRegistry; +use crate::{FeatureFlag, RuntimeGroupSpec}; /// Lists the auto-registered `auth` command under the admin help category so /// it is never uncategorized once clap's auto subcommand list is suppressed. @@ -182,6 +186,7 @@ pub(super) fn ensure_auth_command(cli: &mut Cli) { return; } let mut group = auth_command_group(&default_provider, ®istered_names); + apply_auth_default_fields(&mut group, &cli.config); let mut seen_names: std::collections::HashSet = group.commands.iter().map(|c| c.spec.name.clone()).collect(); for extra in cli.config.auth_extra_commands.clone() { @@ -202,6 +207,7 @@ pub(super) fn ensure_auth_command(cli: &mut Cli) { &mut cli.middleware.schema_registry, &mut cli.middleware.human_views, ); + register_default_auth_views(&mut cli.middleware.human_views); let mut prefix = Vec::new(); group.register_commands(&mut prefix, &mut cli.commands); let mut prefix = Vec::new(); @@ -221,6 +227,29 @@ pub(super) fn ensure_auth_command(cli: &mut Cli) { register_auth_help_entry(cli); } +/// Applies the consumer's default `--fields` to the built-in auth commands. +fn apply_auth_default_fields(group: &mut RuntimeGroupSpec, config: &CliConfig) { + for command in &mut group.commands { + let fields = match command.spec.name.as_str() { + "status" => config.auth_status_default_fields.as_ref(), + "login" => config.auth_login_default_fields.as_ref(), + _ => None, + }; + if let Some(fields) = fields { + command.spec = command.spec.clone().with_default_fields(fields.clone()); + } + } +} + +/// Registers the engine's default `auth status`/`auth login` views. +fn register_default_auth_views(views: &mut HumanViewRegistry) { + for default in [default_auth_status_view(), default_auth_login_view()] { + if !views.has_view(&default.schema_id) { + views.register(default); + } + } +} + /// Mounts the built-in `config` command group and files it under the admin /// help category. Idempotent and yields to a consumer-defined `config` /// subcommand if one already exists. diff --git a/cli-engine/src/cli/schema_tree.rs b/cli-engine/src/cli/schema_tree.rs index 86c76db..ea7243d 100644 --- a/cli-engine/src/cli/schema_tree.rs +++ b/cli-engine/src/cli/schema_tree.rs @@ -362,6 +362,11 @@ fn apply_fields_arg( let mut help = String::from( "Comma-separated fields to include in output (use 'all' or '*' for everything)", ); + // Clap appends its own `[default: ...]` after the last line of help, which + // lands on the end of the field table's final row; render it up front instead. + if let Some(default_value) = default_value { + help.push_str(&format!(" [default: {default_value}]")); + } if let Some(table) = &table { help.push_str("\n\n"); help.push_str(table.trim_end()); @@ -377,7 +382,9 @@ fn apply_fields_arg( .display_order(crate::flags::global_flag_order::FIELDS) .help(help); if let Some(default_value) = default_value { - arg = arg.default_value(default_value.to_owned()); + arg = arg + .default_value(default_value.to_owned()) + .hide_default_value(true); } command.arg(arg) } diff --git a/cli-engine/src/flags/register.rs b/cli-engine/src/flags/register.rs index 5f71ce6..71b50b6 100644 --- a/cli-engine/src/flags/register.rs +++ b/cli-engine/src/flags/register.rs @@ -149,7 +149,7 @@ pub fn register_global_flags(command: Command) -> Command { .action(ArgAction::SetTrue) .conflicts_with("non-interactive") .display_order(global_flag_order::INTERACTIVE) - .help("Force interactive prompts for missing inputs (default when TTY is detected)"), + .help("Force interactive prompts for missing inputs"), ) .arg( Arg::new("non-interactive") diff --git a/cli-engine/src/lib.rs b/cli-engine/src/lib.rs index f8f91a0..a679e7c 100644 --- a/cli-engine/src/lib.rs +++ b/cli-engine/src/lib.rs @@ -122,8 +122,9 @@ pub use auth::storage::{ CredentialKey, CredentialStorage, FileStorage, default_storage, storage_for, }; pub use auth::{ - AuthLoginResult, AuthProvider, AuthStatusEntry, CACHE_TTL, Credential, CredentialRequest, - Dispatcher, SingleProvider, StatusEntry, auth_command_group, login_and_build, + AUTH_LOGIN_VIEW_ID, AUTH_STATUS_VIEW_ID, AuthLoginResult, AuthProvider, AuthStatusEntry, + CACHE_TTL, Credential, CredentialRequest, Dispatcher, SingleProvider, StatusEntry, + auth_command_group, default_auth_login_view, default_auth_status_view, login_and_build, login_and_build_with_scopes, logout_result, status_result, to_status_entry, }; pub use cli::{ diff --git a/cli-engine/tests/auth_views.rs b/cli-engine/tests/auth_views.rs new file mode 100644 index 0000000..233dffc --- /dev/null +++ b/cli-engine/tests/auth_views.rs @@ -0,0 +1,249 @@ +//! Default and overridden human views for the built-in `auth status` and +//! `auth login` commands. + +use std::sync::Arc; + +use async_trait::async_trait; +use cli_engine::{ + AuthProvider, Cli, CliConfig, Credential, HumanViewDef, Result, TableColumn, + default_auth_status_view, +}; +use serde_json::Value; + +#[derive(Debug)] +struct FakeProvider; + +#[async_trait] +impl AuthProvider for FakeProvider { + fn name(&self) -> &str { + "primary" + } + + async fn get_credential(&self, env: &str, _command: &str, _tier: &str) -> Result { + Ok(Credential { + token: "token".to_owned(), + expires_at: "2099-01-01T00:00:00Z".to_owned(), + env: env.to_owned(), + identity: "tester".to_owned(), + ..Credential::default() + }) + } + + async fn status(&self, env: &str) -> Result { + self.get_credential(env, "", "").await + } + + async fn logout(&self, _env: &str) -> Result<()> { + Ok(()) + } + + async fn list_environments(&self) -> Result> { + Ok(vec!["prod".to_owned()]) + } +} + +fn config() -> CliConfig { + CliConfig { + name: "my-cli".to_owned(), + short: "Auth view test CLI".to_owned(), + app_id: "my-cli".to_owned(), + default_auth_provider: Some("primary".to_owned()), + ..CliConfig::default() + } +} + +fn cli_with(config: CliConfig) -> Cli { + let mut cli = Cli::new(config); + cli.register_auth_provider(Arc::new(FakeProvider)); + cli +} + +/// Column headers of the first table-looking line in human output. +fn header_line(rendered: &str) -> &str { + rendered + .lines() + .find(|line| line.contains("IDENTITY")) + .expect("human output has an IDENTITY header line") +} + +fn position(header: &str, column: &str) -> usize { + assert!(header.contains(column), "{column} missing from: {header}"); + header.find(column).expect("checked above") +} + +async fn run(cli: &Cli, args: &[&str]) -> String { + let mut argv = vec!["my-cli"]; + argv.extend_from_slice(args); + let output = cli.run(argv).await; + assert_eq!(output.exit_code, 0, "{}", output.rendered); + output.rendered +} + +#[tokio::test] +async fn default_status_view_leads_with_identity_then_expires() { + let cli = cli_with(config()); + let rendered = run(&cli, &["auth", "status", "--output", "human"]).await; + let header = header_line(&rendered); + assert!(position(header, "IDENTITY") < position(header, "EXPIRES")); + assert!(position(header, "EXPIRES") < position(header, "ENV")); + assert!(header.contains("PROVIDER")); +} + +#[tokio::test] +async fn default_login_view_leads_with_identity_then_expires() { + let cli = cli_with(config()); + let rendered = run( + &cli, + &["auth", "login", "--env", "prod", "--output", "human"], + ) + .await; + assert!(rendered.contains("tester"), "{rendered}"); + let identity = position(&rendered, "Identity"); + let expires = position(&rendered, "Expires"); + assert!(identity < expires, "{rendered}"); +} + +#[tokio::test] +async fn config_view_overrides_status_columns_without_replacing_the_group() { + let columns = default_auth_status_view() + .columns + .into_iter() + .filter(|column| column.field != "provider") + .collect::>(); + let cli = cli_with(config().with_view(HumanViewDef::new("auth:status", columns))); + + let rendered = run(&cli, &["auth", "status", "--output", "human"]).await; + assert!(!header_line(&rendered).contains("PROVIDER"), "{rendered}"); + + // The built-in group is intact: login/logout still resolve. + run(&cli, &["auth", "logout", "--env", "prod"]).await; +} + +#[tokio::test] +async fn override_survives_a_later_provider_registration() { + let cli_config = config().with_view(HumanViewDef::new( + "auth:status", + vec![TableColumn::new("identity", "Identity")], + )); + let mut cli = Cli::new(cli_config); + cli.register_auth_provider(Arc::new(FakeProvider)); + + let rendered = run(&cli, &["auth", "status", "--output", "human"]).await; + assert!(!rendered.contains("EXPIRES"), "{rendered}"); +} + +#[tokio::test] +async fn json_output_still_carries_every_field() { + let cli = cli_with(config()); + let rendered = run(&cli, &["auth", "status", "--output", "json"]).await; + let envelope: Value = serde_json::from_str(&rendered).expect("valid json"); + let row = &envelope["data"][0]; + for field in [ + "provider", + "env", + "identity", + "expires_at", + "expired", + "refreshable", + ] { + assert!(row.get(field).is_some(), "missing {field}: {row}"); + } +} + +#[tokio::test] +async fn fields_flag_narrows_the_default_view() { + let cli = cli_with(config()); + let rendered = run( + &cli, + &[ + "auth", "status", "--output", "human", "--fields", "identity", + ], + ) + .await; + assert!(!rendered.contains("EXPIRES"), "{rendered}"); +} + +const NO_PROVIDER: &str = "identity,expires_at,env,expired,refreshable,scopes"; + +#[tokio::test] +async fn default_fields_leave_provider_out_but_keep_it_selectable() { + let cli = cli_with(config().with_auth_status_default_fields(NO_PROVIDER)); + + let default = run(&cli, &["auth", "status", "--output", "human"]).await; + assert!(!header_line(&default).contains("PROVIDER"), "{default}"); + + let explicit = run( + &cli, + &[ + "auth", + "status", + "--output", + "human", + "--fields", + "identity,provider", + ], + ) + .await; + let header = header_line(&explicit); + assert!(position(header, "IDENTITY") < position(header, "PROVIDER")); + assert!(!header.contains("EXPIRES"), "{explicit}"); +} + +#[tokio::test] +async fn login_default_fields_leave_provider_out_but_keep_it_selectable() { + let cli = cli_with(config().with_auth_login_default_fields("identity,expires_at,env")); + let login = ["auth", "login", "--env", "prod", "--output", "human"]; + + let default = run(&cli, &login).await; + assert!(!default.contains("Provider"), "{default}"); + + let mut explicit = login.to_vec(); + explicit.extend(["--fields", "provider"]); + let explicit = run(&cli, &explicit).await; + assert!(explicit.contains("Provider: primary"), "{explicit}"); +} + +#[tokio::test] +async fn default_fields_project_json_like_any_other_command() { + let cli = cli_with(config().with_auth_status_default_fields(NO_PROVIDER)); + + let default = run(&cli, &["auth", "status", "--output", "json"]).await; + let envelope: Value = serde_json::from_str(&default).expect("valid json"); + assert!(envelope["data"][0].get("provider").is_none(), "{default}"); + + let explicit = run( + &cli, + &["auth", "status", "--output", "json", "--fields", "provider"], + ) + .await; + let envelope: Value = serde_json::from_str(&explicit).expect("valid json"); + assert_eq!(envelope["data"][0]["provider"], "primary"); +} + +#[tokio::test] +async fn help_lists_every_output_field_and_marks_the_defaults() { + let cli = cli_with(config().with_auth_status_default_fields(NO_PROVIDER)); + let help = run(&cli, &["auth", "status", "--help"]).await; + assert!(help.contains("Output fields:"), "{help}"); + let provider = help + .lines() + .find(|line| line.trim_start().starts_with("provider")) + .expect("provider is listed"); + assert!(!provider.contains("(default)"), "{provider}"); + let identity = help + .lines() + .find(|line| line.trim_start().starts_with("identity")) + .expect("identity is listed"); + assert!(identity.contains("(default)"), "{identity}"); +} + +#[tokio::test] +async fn fields_default_precedes_the_field_table() { + let cli = cli_with(config().with_auth_status_default_fields(NO_PROVIDER)); + let help = run(&cli, &["auth", "status", "--help"]).await; + let default = format!("[default: {NO_PROVIDER}]"); + assert_eq!(help.matches("[default: identity").count(), 1, "{help}"); + let default_at = help.find(&default).expect("default is shown"); + let table_at = help.find("Output fields:").expect("table is shown"); + assert!(default_at < table_at, "{help}"); +} diff --git a/cli-engine/tests/interactivity.rs b/cli-engine/tests/interactivity.rs index c31d11f..c548a73 100644 --- a/cli-engine/tests/interactivity.rs +++ b/cli-engine/tests/interactivity.rs @@ -136,3 +136,26 @@ async fn interactive_and_non_interactive_conflict() { output.rendered ); } + +/// The `--interactive` help line from `--help` output. +async fn interactive_help(config: CliConfig) -> String { + let output = Cli::new(config).run(["test-cli", "tree", "--help"]).await; + output + .rendered + .lines() + .find(|line| line.contains("--interactive")) + .unwrap_or_default() + .to_owned() +} + +#[tokio::test] +async fn interactive_help_claims_tty_default_only_when_auto_interactive() { + let base = || CliConfig::new("test-cli", "Test CLI", "test-cli"); + + let manual = interactive_help(base()).await; + assert!(manual.contains("Force interactive prompts"), "{manual}"); + assert!(!manual.contains("default"), "{manual}"); + + let auto = interactive_help(base().with_auto_interactive(true)).await; + assert!(auto.contains("default when a TTY is detected"), "{auto}"); +} From ba29d1b1764ac2049f72f28bc7ecc8550aae973b Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 12:02:22 -0700 Subject: [PATCH 2/6] fix(output): fall back to generic rendering when a view matches none of the data A fixed human view rendered an all-blank table when `--expr` reshaped the result into fields the view never names (e.g. `auth status --expr '{user: identity}'`). Render such data generically instead. Only data that is objects, none of which resolve any of the view's columns, falls back; a view that matches at least one column keeps rendering as before. Refs DEVEX-1267 Co-Authored-By: Claude Sonnet 5.5 --- cli-engine/src/output/human/columns.rs | 22 ++++++++++++ cli-engine/src/output/human/mod.rs | 12 +++++++ cli-engine/tests/auth_views.rs | 50 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/cli-engine/src/output/human/columns.rs b/cli-engine/src/output/human/columns.rs index 77f5443..01b69fe 100644 --- a/cli-engine/src/output/human/columns.rs +++ b/cli-engine/src/output/human/columns.rs @@ -26,6 +26,28 @@ pub(crate) fn terminal_width() -> usize { } } +/// Whether a registered view can show anything of `data`. +/// +/// `false` only when `data` is an object, or a non-empty array of objects, +/// and none of the view's columns resolve in any of them — the shape of a +/// result `--expr` has reshaped into fields the view never names, which the +/// view would render as an all-blank table. Anything else (no data, scalars, +/// an empty array, at least one resolving column) keeps the view. +pub(super) fn view_applies_to(data: &Value, columns: &[TableColumn]) -> bool { + let resolves = |object: &serde_json::Map| { + columns + .iter() + .any(|column| resolve_field_path(object, &column.field).is_some()) + }; + match data { + Value::Object(object) => resolves(object), + Value::Array(items) if items.iter().all(Value::is_object) && !items.is_empty() => { + items.iter().filter_map(Value::as_object).any(resolves) + } + _ => true, + } +} + /// Builds the column catalog for data with no registered view: when `fields` /// names specific fields (not empty/`all`/`*`), columns are derived from that /// list, in the order given (deduplicated) — the same order source a diff --git a/cli-engine/src/output/human/mod.rs b/cli-engine/src/output/human/mod.rs index a843b14..d799f31 100644 --- a/cli-engine/src/output/human/mod.rs +++ b/cli-engine/src/output/human/mod.rs @@ -16,6 +16,7 @@ mod tests; mod value_format; use body::render_data_body; +use columns::view_applies_to; use footer::{ append_cursor_summary, append_next_actions, append_pagination_summary, append_render_notes, }; @@ -377,6 +378,17 @@ pub(crate) fn render_human_with_registry_selected( return output; } match registry.columns(schema_id) { + // A view that names none of the data's fields (`--expr` reshaped it) + // would render blank; show the data generically instead, ignoring the + // `--fields` selection, which was written against the view. + Some(columns) + if envelope + .data + .as_ref() + .is_some_and(|data| !view_applies_to(data, columns)) => + { + render_human_with_view(envelope, None, "", false) + } Some(columns) => { let selected = select_columns(columns, fields); render_human_with_view(envelope, Some(&selected), fields, fields_explicit) diff --git a/cli-engine/tests/auth_views.rs b/cli-engine/tests/auth_views.rs index 233dffc..c832e81 100644 --- a/cli-engine/tests/auth_views.rs +++ b/cli-engine/tests/auth_views.rs @@ -247,3 +247,53 @@ async fn fields_default_precedes_the_field_table() { let table_at = help.find("Output fields:").expect("table is shown"); assert!(default_at < table_at, "{help}"); } + +#[tokio::test] +async fn expr_reshaped_results_fall_back_to_generic_rendering() { + let cli = cli_with(config()); + let login = ["auth", "login", "--env", "prod"]; + let status = ["auth", "status", "--provider", "primary", "--env", "prod"]; + for (base, expr) in [ + (&status[..], "{user: identity}"), + (&login[..], "{user: identity}"), + ] { + let mut args = base.to_vec(); + args.extend(["--expr", expr, "--output", "human"]); + let rendered = run(&cli, &args).await; + assert!(rendered.to_lowercase().contains("user"), "{rendered}"); + assert!(rendered.contains("tester"), "{rendered}"); + } + + let rows = run( + &cli, + &[ + "auth", + "status", + "--expr", + "[].{user: identity}", + "--output", + "human", + ], + ) + .await; + assert!(rows.contains("USER") && rows.contains("tester"), "{rows}"); +} + +#[tokio::test] +async fn expr_that_keeps_the_shape_keeps_the_view() { + let cli = cli_with(config()); + let rendered = run( + &cli, + &[ + "auth", + "status", + "--expr", + "[?env=='prod']", + "--output", + "human", + ], + ) + .await; + let header = header_line(&rendered); + assert!(position(header, "IDENTITY") < position(header, "EXPIRES")); +} From b1b3b99d26d51180fc72fa5d2b7d602f14780bae Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 12:14:50 -0700 Subject: [PATCH 3/6] fix(output): limit the generic-render fallback to expr-reshaped results A view whose columns are all optional would fall back to generic rendering for an ordinary response that merely lacked those fields, dropping the view. Fall back only when `--expr` reshaped the data. Refs DEVEX-1267 Co-Authored-By: Claude Sonnet 5.5 --- cli-engine/src/middleware/run.rs | 1 + cli-engine/src/output/human/mod.rs | 21 +++++++---- .../src/output/human/tests/field_selection.rs | 11 +++++- cli-engine/src/output/human/tests/registry.rs | 37 ++++++++++++++++++- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/cli-engine/src/middleware/run.rs b/cli-engine/src/middleware/run.rs index 9fbfad1..c7a5efb 100644 --- a/cli-engine/src/middleware/run.rs +++ b/cli-engine/src/middleware/run.rs @@ -676,6 +676,7 @@ impl Middleware { view_id, effective_fields, self.fields_explicit, + !self.expr.is_empty(), ) } else { crate::output::render(output_format, &prepared)? diff --git a/cli-engine/src/output/human/mod.rs b/cli-engine/src/output/human/mod.rs index d799f31..5f6ce98 100644 --- a/cli-engine/src/output/human/mod.rs +++ b/cli-engine/src/output/human/mod.rs @@ -359,6 +359,11 @@ pub fn preview_human_view(data: impl Serialize, columns: &[TableColumn]) -> Stri /// selected columns, since the user named exactly what they want to see; the /// row overflows the terminal rather than losing or shortening a column. /// `NO_TRUNCATE_MAX_WIDTH` still caps a pathologically long value either way. +/// +/// `reshaped` is `true` when `--expr` reshaped the data. Only a reshaped +/// result can fall back to generic rendering when the view names none of its +/// fields; an ordinary response that merely lacks the view's optional fields +/// keeps the view. #[must_use] pub(crate) fn render_human_with_registry_selected( envelope: &Envelope, @@ -366,6 +371,7 @@ pub(crate) fn render_human_with_registry_selected( schema_id: &str, fields: &str, fields_explicit: bool, + reshaped: bool, ) -> String { if let Some(error) = &envelope.error { return format!("Error: {}\n", error.message); @@ -378,14 +384,15 @@ pub(crate) fn render_human_with_registry_selected( return output; } match registry.columns(schema_id) { - // A view that names none of the data's fields (`--expr` reshaped it) - // would render blank; show the data generically instead, ignoring the - // `--fields` selection, which was written against the view. + // A view that names none of the data's fields because `--expr` + // reshaped it would render blank; show the data generically instead, + // ignoring the `--fields` selection, which was written against the view. Some(columns) - if envelope - .data - .as_ref() - .is_some_and(|data| !view_applies_to(data, columns)) => + if reshaped + && envelope + .data + .as_ref() + .is_some_and(|data| !view_applies_to(data, columns)) => { render_human_with_view(envelope, None, "", false) } diff --git a/cli-engine/src/output/human/tests/field_selection.rs b/cli-engine/src/output/human/tests/field_selection.rs index 8116774..c50811e 100644 --- a/cli-engine/src/output/human/tests/field_selection.rs +++ b/cli-engine/src/output/human/tests/field_selection.rs @@ -91,6 +91,7 @@ fn no_view_array_rendering_follows_requested_field_order() { "domain:suggest", "domain,price1Year,currency", true, + false, ); let header_line = rendered.lines().next().expect("header line"); @@ -120,8 +121,14 @@ fn registered_view_rendering_follows_requested_field_order() { "things", ); - let rendered = - render_human_with_registry_selected(&envelope, ®istry, "things", "status,id", true); + let rendered = render_human_with_registry_selected( + &envelope, + ®istry, + "things", + "status,id", + true, + false, + ); let header_line = rendered.lines().next().expect("header line"); assert!(!header_line.contains("NAME"), "{rendered}"); diff --git a/cli-engine/src/output/human/tests/registry.rs b/cli-engine/src/output/human/tests/registry.rs index 2dff969..80e0674 100644 --- a/cli-engine/src/output/human/tests/registry.rs +++ b/cli-engine/src/output/human/tests/registry.rs @@ -24,7 +24,14 @@ fn custom_human_output_appends_next_steps_footer() { .with_param("cart-id", NextActionParam::value("cart-1")), ]); - let out = render_human_with_registry_selected(&envelope, ®istry, "shopping-cart", "", false); + let out = render_human_with_registry_selected( + &envelope, + ®istry, + "shopping-cart", + "", + false, + false, + ); assert!(out.starts_with("Cart: ready\n"), "{out}"); assert!(out.contains("\nNext steps:\n"), "{out}"); @@ -52,7 +59,33 @@ fn human_view_registry_custom_renderer_wins_over_columns() { }); let envelope = Envelope::success(json!({"name": "alpha"}), "things"); - let rendered = render_human_with_registry_selected(&envelope, ®istry, "things", "", false); + let rendered = + render_human_with_registry_selected(&envelope, ®istry, "things", "", false, false); assert_eq!(rendered, "custom:alpha\n"); } + +#[test] +fn view_naming_none_of_the_data_falls_back_only_when_expr_reshaped_it() { + let mut registry = HumanViewRegistry::new(); + registry.register(HumanViewDef::new( + "things", + vec![TableColumn::new("name", "Name")], + )); + // An ordinary response that merely lacks the view's optional field keeps + // the view; the same shape after `--expr` is shown generically. + let envelope = Envelope::success(json!({"id": "1"}), "things"); + + let ordinary = + render_human_with_registry_selected(&envelope, ®istry, "things", "", false, false); + assert!(ordinary.contains("Name"), "{ordinary}"); + assert!(!ordinary.contains("Id"), "{ordinary}"); + + let reshaped = + render_human_with_registry_selected(&envelope, ®istry, "things", "", false, true); + assert!( + reshaped.contains("Id") || reshaped.contains("id"), + "{reshaped}" + ); + assert!(!reshaped.contains("Name"), "{reshaped}"); +} From 5117d05e8813785b18c427ca8fe4ea20139be9e9 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 12:40:08 -0700 Subject: [PATCH 4/6] fix(output): don't validate --fields against a view when --expr is set `--expr` can reshape the result into any shape, so the view's columns say nothing about what `--fields` may name. Skip the view-catalog check in that case, and let the generic fallback apply an explicit `--fields` to the reshaped data. Refs DEVEX-1267 Co-Authored-By: Claude Sonnet 5.5 --- cli-engine/src/middleware/run.rs | 5 +++- cli-engine/src/output/human/mod.rs | 8 ++++--- cli-engine/tests/auth_views.rs | 38 ++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/cli-engine/src/middleware/run.rs b/cli-engine/src/middleware/run.rs index c7a5efb..8ca616a 100644 --- a/cli-engine/src/middleware/run.rs +++ b/cli-engine/src/middleware/run.rs @@ -512,8 +512,11 @@ impl Middleware { // closes elsewhere. So an explicit `--fields` (never a // `default_fields` fallback — same reasoning as // `PipelineOpts::fields_are_default`) is checked against the view's - // column catalog here instead. + // column catalog here instead. Not under `--expr`: its result can be + // any shape, so the view's columns say nothing about what `--fields` + // may name. if human_view + && self.expr.is_empty() && self.fields_explicit && let Some(columns) = self.human_views.columns(view_id) { diff --git a/cli-engine/src/output/human/mod.rs b/cli-engine/src/output/human/mod.rs index 5f6ce98..027a0a0 100644 --- a/cli-engine/src/output/human/mod.rs +++ b/cli-engine/src/output/human/mod.rs @@ -385,8 +385,9 @@ pub(crate) fn render_human_with_registry_selected( } match registry.columns(schema_id) { // A view that names none of the data's fields because `--expr` - // reshaped it would render blank; show the data generically instead, - // ignoring the `--fields` selection, which was written against the view. + // reshaped it would render blank; show the data generically instead. + // Only an explicit `--fields` carries over (it can name the reshaped + // fields); a command's default fields were written against the view. Some(columns) if reshaped && envelope @@ -394,7 +395,8 @@ pub(crate) fn render_human_with_registry_selected( .as_ref() .is_some_and(|data| !view_applies_to(data, columns)) => { - render_human_with_view(envelope, None, "", false) + let fields = if fields_explicit { fields } else { "" }; + render_human_with_view(envelope, None, fields, fields_explicit) } Some(columns) => { let selected = select_columns(columns, fields); diff --git a/cli-engine/tests/auth_views.rs b/cli-engine/tests/auth_views.rs index c832e81..e6db819 100644 --- a/cli-engine/tests/auth_views.rs +++ b/cli-engine/tests/auth_views.rs @@ -297,3 +297,41 @@ async fn expr_that_keeps_the_shape_keeps_the_view() { let header = header_line(&rendered); assert!(position(header, "IDENTITY") < position(header, "EXPIRES")); } + +#[tokio::test] +async fn fields_after_expr_name_the_reshaped_result_not_the_view() { + let cli = cli_with(config()); + let status = ["auth", "status", "--provider", "primary", "--env", "prod"]; + let login = ["auth", "login", "--env", "prod"]; + for base in [&status[..], &login[..]] { + let mut args = base.to_vec(); + args.extend([ + "--expr", + "{user: identity, where: env}", + "--fields", + "user", + "--output", + "human", + ]); + let rendered = run(&cli, &args).await; + assert!(rendered.to_lowercase().contains("user"), "{rendered}"); + assert!(rendered.contains("tester"), "{rendered}"); + assert!(!rendered.to_lowercase().contains("where"), "{rendered}"); + } +} + +#[tokio::test] +async fn fields_without_expr_still_reject_names_the_view_lacks() { + let cli = cli_with(config()); + let output = cli + .run([ + "my-cli", "auth", "status", "--fields", "nope", "--output", "human", + ]) + .await; + assert_ne!(output.exit_code, 0); + assert!( + output.rendered.contains("unknown field"), + "{}", + output.rendered + ); +} From 0eb2b2cbed649cdf544c1d598136c4853bcf241f Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 13:08:30 -0700 Subject: [PATCH 5/6] fix(output): render --expr results generically instead of through a view `--expr` hands the caller control of the result's shape, so a registered view's columns no longer describe it. Treat the command as having no view in human output when `--expr` is set; the existing pipeline then validates and projects `--fields` against the expression's result, as it does for JSON. This replaces the earlier fallback heuristic and skipped validation, both of which left edge cases (typos accepted, explicit fields dropped when the result kept one view column). `--filter` still keeps the view. Refs DEVEX-1267 Co-Authored-By: Claude Sonnet 5.5 --- cli-engine/src/middleware/run.rs | 11 +-- cli-engine/src/output/human/columns.rs | 22 ------ cli-engine/src/output/human/mod.rs | 21 ------ .../src/output/human/tests/field_selection.rs | 11 +-- cli-engine/src/output/human/tests/registry.rs | 37 +--------- cli-engine/tests/auth_views.rs | 68 +++++++++++-------- 6 files changed, 50 insertions(+), 120 deletions(-) diff --git a/cli-engine/src/middleware/run.rs b/cli-engine/src/middleware/run.rs index 8ca616a..bef4339 100644 --- a/cli-engine/src/middleware/run.rs +++ b/cli-engine/src/middleware/run.rs @@ -502,6 +502,11 @@ impl Middleware { } else { default_fields }; + // `--expr` hands the caller control of the result's shape, so the + // view's columns no longer describe it: render generically, and let + // the pipeline validate and project `--fields` against the result, + // as it does for JSON. + let view_id = if self.expr.is_empty() { view_id } else { "" }; let human_view = output_format == OutputFormat::Human && self.human_views.has_view(view_id); // `apply_pipeline` never sees `effective_fields` for a registered // view (`projection_fields` below is forced to `""` so the view reads @@ -512,11 +517,8 @@ impl Middleware { // closes elsewhere. So an explicit `--fields` (never a // `default_fields` fallback — same reasoning as // `PipelineOpts::fields_are_default`) is checked against the view's - // column catalog here instead. Not under `--expr`: its result can be - // any shape, so the view's columns say nothing about what `--fields` - // may name. + // column catalog here instead. if human_view - && self.expr.is_empty() && self.fields_explicit && let Some(columns) = self.human_views.columns(view_id) { @@ -679,7 +681,6 @@ impl Middleware { view_id, effective_fields, self.fields_explicit, - !self.expr.is_empty(), ) } else { crate::output::render(output_format, &prepared)? diff --git a/cli-engine/src/output/human/columns.rs b/cli-engine/src/output/human/columns.rs index 01b69fe..77f5443 100644 --- a/cli-engine/src/output/human/columns.rs +++ b/cli-engine/src/output/human/columns.rs @@ -26,28 +26,6 @@ pub(crate) fn terminal_width() -> usize { } } -/// Whether a registered view can show anything of `data`. -/// -/// `false` only when `data` is an object, or a non-empty array of objects, -/// and none of the view's columns resolve in any of them — the shape of a -/// result `--expr` has reshaped into fields the view never names, which the -/// view would render as an all-blank table. Anything else (no data, scalars, -/// an empty array, at least one resolving column) keeps the view. -pub(super) fn view_applies_to(data: &Value, columns: &[TableColumn]) -> bool { - let resolves = |object: &serde_json::Map| { - columns - .iter() - .any(|column| resolve_field_path(object, &column.field).is_some()) - }; - match data { - Value::Object(object) => resolves(object), - Value::Array(items) if items.iter().all(Value::is_object) && !items.is_empty() => { - items.iter().filter_map(Value::as_object).any(resolves) - } - _ => true, - } -} - /// Builds the column catalog for data with no registered view: when `fields` /// names specific fields (not empty/`all`/`*`), columns are derived from that /// list, in the order given (deduplicated) — the same order source a diff --git a/cli-engine/src/output/human/mod.rs b/cli-engine/src/output/human/mod.rs index 027a0a0..a843b14 100644 --- a/cli-engine/src/output/human/mod.rs +++ b/cli-engine/src/output/human/mod.rs @@ -16,7 +16,6 @@ mod tests; mod value_format; use body::render_data_body; -use columns::view_applies_to; use footer::{ append_cursor_summary, append_next_actions, append_pagination_summary, append_render_notes, }; @@ -359,11 +358,6 @@ pub fn preview_human_view(data: impl Serialize, columns: &[TableColumn]) -> Stri /// selected columns, since the user named exactly what they want to see; the /// row overflows the terminal rather than losing or shortening a column. /// `NO_TRUNCATE_MAX_WIDTH` still caps a pathologically long value either way. -/// -/// `reshaped` is `true` when `--expr` reshaped the data. Only a reshaped -/// result can fall back to generic rendering when the view names none of its -/// fields; an ordinary response that merely lacks the view's optional fields -/// keeps the view. #[must_use] pub(crate) fn render_human_with_registry_selected( envelope: &Envelope, @@ -371,7 +365,6 @@ pub(crate) fn render_human_with_registry_selected( schema_id: &str, fields: &str, fields_explicit: bool, - reshaped: bool, ) -> String { if let Some(error) = &envelope.error { return format!("Error: {}\n", error.message); @@ -384,20 +377,6 @@ pub(crate) fn render_human_with_registry_selected( return output; } match registry.columns(schema_id) { - // A view that names none of the data's fields because `--expr` - // reshaped it would render blank; show the data generically instead. - // Only an explicit `--fields` carries over (it can name the reshaped - // fields); a command's default fields were written against the view. - Some(columns) - if reshaped - && envelope - .data - .as_ref() - .is_some_and(|data| !view_applies_to(data, columns)) => - { - let fields = if fields_explicit { fields } else { "" }; - render_human_with_view(envelope, None, fields, fields_explicit) - } Some(columns) => { let selected = select_columns(columns, fields); render_human_with_view(envelope, Some(&selected), fields, fields_explicit) diff --git a/cli-engine/src/output/human/tests/field_selection.rs b/cli-engine/src/output/human/tests/field_selection.rs index c50811e..8116774 100644 --- a/cli-engine/src/output/human/tests/field_selection.rs +++ b/cli-engine/src/output/human/tests/field_selection.rs @@ -91,7 +91,6 @@ fn no_view_array_rendering_follows_requested_field_order() { "domain:suggest", "domain,price1Year,currency", true, - false, ); let header_line = rendered.lines().next().expect("header line"); @@ -121,14 +120,8 @@ fn registered_view_rendering_follows_requested_field_order() { "things", ); - let rendered = render_human_with_registry_selected( - &envelope, - ®istry, - "things", - "status,id", - true, - false, - ); + let rendered = + render_human_with_registry_selected(&envelope, ®istry, "things", "status,id", true); let header_line = rendered.lines().next().expect("header line"); assert!(!header_line.contains("NAME"), "{rendered}"); diff --git a/cli-engine/src/output/human/tests/registry.rs b/cli-engine/src/output/human/tests/registry.rs index 80e0674..2dff969 100644 --- a/cli-engine/src/output/human/tests/registry.rs +++ b/cli-engine/src/output/human/tests/registry.rs @@ -24,14 +24,7 @@ fn custom_human_output_appends_next_steps_footer() { .with_param("cart-id", NextActionParam::value("cart-1")), ]); - let out = render_human_with_registry_selected( - &envelope, - ®istry, - "shopping-cart", - "", - false, - false, - ); + let out = render_human_with_registry_selected(&envelope, ®istry, "shopping-cart", "", false); assert!(out.starts_with("Cart: ready\n"), "{out}"); assert!(out.contains("\nNext steps:\n"), "{out}"); @@ -59,33 +52,7 @@ fn human_view_registry_custom_renderer_wins_over_columns() { }); let envelope = Envelope::success(json!({"name": "alpha"}), "things"); - let rendered = - render_human_with_registry_selected(&envelope, ®istry, "things", "", false, false); + let rendered = render_human_with_registry_selected(&envelope, ®istry, "things", "", false); assert_eq!(rendered, "custom:alpha\n"); } - -#[test] -fn view_naming_none_of_the_data_falls_back_only_when_expr_reshaped_it() { - let mut registry = HumanViewRegistry::new(); - registry.register(HumanViewDef::new( - "things", - vec![TableColumn::new("name", "Name")], - )); - // An ordinary response that merely lacks the view's optional field keeps - // the view; the same shape after `--expr` is shown generically. - let envelope = Envelope::success(json!({"id": "1"}), "things"); - - let ordinary = - render_human_with_registry_selected(&envelope, ®istry, "things", "", false, false); - assert!(ordinary.contains("Name"), "{ordinary}"); - assert!(!ordinary.contains("Id"), "{ordinary}"); - - let reshaped = - render_human_with_registry_selected(&envelope, ®istry, "things", "", false, true); - assert!( - reshaped.contains("Id") || reshaped.contains("id"), - "{reshaped}" - ); - assert!(!reshaped.contains("Name"), "{reshaped}"); -} diff --git a/cli-engine/tests/auth_views.rs b/cli-engine/tests/auth_views.rs index e6db819..f3ee543 100644 --- a/cli-engine/tests/auth_views.rs +++ b/cli-engine/tests/auth_views.rs @@ -249,19 +249,17 @@ async fn fields_default_precedes_the_field_table() { } #[tokio::test] -async fn expr_reshaped_results_fall_back_to_generic_rendering() { +async fn expr_renders_the_result_generically() { let cli = cli_with(config()); - let login = ["auth", "login", "--env", "prod"]; let status = ["auth", "status", "--provider", "primary", "--env", "prod"]; - for (base, expr) in [ - (&status[..], "{user: identity}"), - (&login[..], "{user: identity}"), - ] { + let login = ["auth", "login", "--env", "prod"]; + for base in [&status[..], &login[..]] { let mut args = base.to_vec(); - args.extend(["--expr", expr, "--output", "human"]); + args.extend(["--expr", "{user: identity}", "--output", "human"]); let rendered = run(&cli, &args).await; assert!(rendered.to_lowercase().contains("user"), "{rendered}"); assert!(rendered.contains("tester"), "{rendered}"); + assert!(!rendered.contains("Provider"), "{rendered}"); } let rows = run( @@ -280,34 +278,35 @@ async fn expr_reshaped_results_fall_back_to_generic_rendering() { } #[tokio::test] -async fn expr_that_keeps_the_shape_keeps_the_view() { +async fn filter_keeps_the_view() { let cli = cli_with(config()); - let rendered = run( + let filtered = run( &cli, &[ "auth", "status", - "--expr", - "[?env=='prod']", + "--filter", + "env=='prod'", "--output", "human", ], ) .await; - let header = header_line(&rendered); + let header = header_line(&filtered); assert!(position(header, "IDENTITY") < position(header, "EXPIRES")); } #[tokio::test] -async fn fields_after_expr_name_the_reshaped_result_not_the_view() { +async fn fields_after_expr_select_from_the_expression_result() { let cli = cli_with(config()); let status = ["auth", "status", "--provider", "primary", "--env", "prod"]; let login = ["auth", "login", "--env", "prod"]; for base in [&status[..], &login[..]] { + // The expression keeps `identity` (a view column) and adds `user`. let mut args = base.to_vec(); args.extend([ "--expr", - "{user: identity, where: env}", + "{identity: identity, user: env}", "--fields", "user", "--output", @@ -315,23 +314,36 @@ async fn fields_after_expr_name_the_reshaped_result_not_the_view() { ]); let rendered = run(&cli, &args).await; assert!(rendered.to_lowercase().contains("user"), "{rendered}"); - assert!(rendered.contains("tester"), "{rendered}"); - assert!(!rendered.to_lowercase().contains("where"), "{rendered}"); + assert!(rendered.contains("prod"), "{rendered}"); + assert!(!rendered.contains("tester"), "{rendered}"); } } #[tokio::test] -async fn fields_without_expr_still_reject_names_the_view_lacks() { +async fn unknown_fields_are_rejected_with_and_without_expr() { let cli = cli_with(config()); - let output = cli - .run([ - "my-cli", "auth", "status", "--fields", "nope", "--output", "human", - ]) - .await; - assert_ne!(output.exit_code, 0); - assert!( - output.rendered.contains("unknown field"), - "{}", - output.rendered - ); + for extra in [ + vec!["--fields", "nope"], + vec!["--expr", "{user: identity}", "--fields", "uesr"], + vec!["--expr", "@", "--fields", "nope"], + ] { + let mut args = vec![ + "my-cli", + "auth", + "status", + "--provider", + "primary", + "--env", + "prod", + ]; + args.extend(extra.iter().copied()); + args.extend(["--output", "human"]); + let output = cli.run(args).await; + assert_ne!(output.exit_code, 0, "{}", output.rendered); + assert!( + output.rendered.contains("unknown field"), + "{}", + output.rendered + ); + } } From b4f3d7aebfcd84dde90ffd60dc2ee617f42625c9 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 20 Oct 2026 15:36:59 -0700 Subject: [PATCH 6/6] fix(output): don't project default_fields onto an --expr result When `--expr` was set without an explicit `--fields`, `effective_fields` still fell back to the command's `default_fields` and projected those names onto the expression's result. Since default-field projection is never validated against the data, names the result lacked were silently dropped, collapsing e.g. `auth status --expr '{user: identity}'` to `{}` / `(no data)` whenever the command had default fields set (as a consumer does to hide `provider`). Fall back to no projection under `--expr` instead. Affects JSON/TOON too, intentionally: default fields describe the command's own output, not the expression's. Refs DEVEX-1267 Co-Authored-By: Claude Opus 4.8 --- cli-engine/src/middleware/run.rs | 15 ++++++++++----- cli-engine/tests/auth_views.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/cli-engine/src/middleware/run.rs b/cli-engine/src/middleware/run.rs index bef4339..0fdb8b0 100644 --- a/cli-engine/src/middleware/run.rs +++ b/cli-engine/src/middleware/run.rs @@ -497,15 +497,20 @@ impl Middleware { // the full payload — the data is not projected, which would // otherwise blank out the kept columns. Everywhere else (JSON/TOON, // or generic human output) it projects the output data. + // `--expr` hands the caller control of the result's shape, so + // neither the command's `default_fields` nor its view describe the + // result any longer. Fall back to no projection (not `default_fields`, + // whose names the expression's result need not contain — which would + // silently project to `{}`), and render generically so an explicit + // `--fields` is validated and projected against the result, as for + // JSON. An explicit `--fields` still applies. `--filter` keeps both. let effective_fields = if self.fields_explicit { self.fields.as_str() - } else { + } else if self.expr.is_empty() { default_fields + } else { + "" }; - // `--expr` hands the caller control of the result's shape, so the - // view's columns no longer describe it: render generically, and let - // the pipeline validate and project `--fields` against the result, - // as it does for JSON. let view_id = if self.expr.is_empty() { view_id } else { "" }; let human_view = output_format == OutputFormat::Human && self.human_views.has_view(view_id); // `apply_pipeline` never sees `effective_fields` for a registered diff --git a/cli-engine/tests/auth_views.rs b/cli-engine/tests/auth_views.rs index f3ee543..4bf6c84 100644 --- a/cli-engine/tests/auth_views.rs +++ b/cli-engine/tests/auth_views.rs @@ -347,3 +347,29 @@ async fn unknown_fields_are_rejected_with_and_without_expr() { ); } } + +#[tokio::test] +async fn expr_ignores_default_fields() { + // With `default_fields` set (how a consumer hides `provider`), a bare + // `--expr` must render its own result, not project the command's default + // fields onto it — otherwise the result's fields are dropped to `{}`. + let cli = cli_with( + config() + .with_auth_status_default_fields(NO_PROVIDER) + .with_auth_login_default_fields("identity,expires_at,env,refreshable,scopes"), + ); + let status = ["auth", "status", "--provider", "primary", "--env", "prod"]; + let login = ["auth", "login", "--env", "prod"]; + for base in [&status[..], &login[..]] { + for out in ["human", "json"] { + let mut args = base.to_vec(); + args.extend(["--expr", "{user: identity}", "--output", out]); + let rendered = run(&cli, &args).await; + assert!( + rendered.to_lowercase().contains("user"), + "{out}: {rendered}" + ); + assert!(rendered.contains("tester"), "{out}: {rendered}"); + } + } +}