diff --git a/AGENTS.md b/AGENTS.md index fc88bbd..f7602ab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -227,6 +227,7 @@ Command checklist: authenticate (this also suppresses default-env injection). Forgetting these annotations only over-prompts; it never lets a gated command run unauthenticated. - Use `.with_tier(...)` or `.mutates(true)` for mutating commands so `--dry-run` can short-circuit them. +- Commands, groups, and modules default to `Stage::Ga` (visible everywhere). Add `.with_feature_flag(key, Stage::Experimental)` (or `Stage::Beta`) only when a command needs extra scrutiny before it reaches a public/external consumer CLI; promoting to GA later is a one-line removal or bump, not a rewrite. - Prefer returning structured JSON values from handlers; let cli-engine render JSON, human, and TOON formats. - Prefer `CommandSpec::from_args::()` + `RuntimeCommandSpec::new_typed` when the command has many flags, needs clap validation attributes, or when porting existing derive-based commands. Use the builder path for simple commands with one or two flags. diff --git a/docs/concepts.md b/docs/concepts.md index c10156b..8bad0e5 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -230,6 +230,7 @@ The framework registers built-in commands for common CLI behavior: | `tree` | Always | Displays the full command hierarchy. | | `auth login` / `auth status` / `auth logout` | Auth providers are registered or a default provider is configured | Manages credentials. | | `guide [topic]` | Guides are registered | Lists and displays embedded guides. | +| `flags list` / `flags info ` | Always, unless a consumer module already registers a top-level `flags` group (the built-in group yields to it) | Inspects declared feature flags and the active policy; see [Feature Flags & Stages](#feature-flags--stages). | `guide` accepts zero or one topic. Additional positional arguments are rejected before guide content is rendered. @@ -420,6 +421,28 @@ Risk tiers classify command impact: `CommandSpec::mutates(true)` also marks a command as dry-run promptable. +## Feature Flags & Stages + +Feature flags classify command readiness rather than risk. `Stage` orders `Experimental < Beta < Ga`; the default is `Ga`, so gating is opt-in — a command with no flag declaration is fully visible under every policy. + +`.with_feature_flag(key, stage)` is available on `CommandSpec`, `GroupSpec`, and `Module`. A node's effective flag is its own declaration if set, else the nearest ancestor's, walking module → group → nested group → command; the nearest declaration wins. A node with no declaration anywhere in its chain resolves to `Stage::Ga` with no key, so existing commands are unaffected unless an author opts a node into a lower stage. + +```rust +use cli_engine::{CommandSpec, Stage}; + +CommandSpec::new("preview", "Preview an upcoming feature") + .with_feature_flag("project-preview", Stage::Experimental); +``` + +`Cli::add_module`/`add_module_group` resolve the cascading flag for every node while building the command tree, record each flagged node into a `FlagRegistry`, and prune any node whose effective flag isn't visible under the active `FlagPolicy`. Pruning removes the node from the tree entirely — help, `--schema`, search, and dispatch — not just from a listing. + +`FlagPolicy` has two fields: `min_stage` (the floor a node's stage must meet or exceed) and `overrides` (per-key stage substitutions, checked before `min_stage`). The policy is assembled from `CliConfig::with_min_stage`/`CliConfig::with_feature_override`, layered with the active environment's own `min_stage`/`feature_overrides` when `with_environments` is configured; see [Environments](environments.md) for the environment-layer precedence and TOML shape. + +The built-in `flags` command group exposes: + +- `flags list` — every declared flag node (path, key, stage, effective visibility). +- `flags info ` — the active policy for one key plus every node that resolved to it, with `decided_by: "override"` or `"min_stage"` indicating which policy layer decided visibility. + ## Output Handlers return JSON-serializable data and a system id. Middleware wraps the result in an envelope diff --git a/docs/environments.md b/docs/environments.md index 3bcff84..65befab 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -37,9 +37,16 @@ client_id = "ote-client-id" auth_url = "https://api.ote.example.com/v2/oauth2/authorize" token_url = "https://api.ote.example.com/v2/oauth2/token" api_url = "https://api.ote.example.com" + +[dev] +min_stage = "experimental" + +[dev.feature_overrides] +"domain-bulk-transfer" = "beta" ``` The recognized OAuth keys — `client_id`, `auth_url`, `token_url`, and `scopes` (an array of strings) — are parsed into the typed `OAuthConfig` slice of the resolved `Environment`. +`min_stage` and the `[.feature_overrides]` table are the feature-flag layer, described in [Feature-Flag Layering](#feature-flag-layering) below. `feature_overrides` (rather than a shorter name like `features`) is deliberately specific: `EnvironmentDef`'s unrecognized keys fall through to the free-form `extra` bag, so a generic name would collide with any existing app-specific `extra` key of the same name. Every other key is captured as a free-form field in `Environment::extra`, which is a `BTreeMap` — so these values **must be TOML strings** (for example `api_url` above). A non-OAuth key whose value is a number, boolean, or array fails to parse; quote it as a string instead. The `extra` bag is printed verbatim by `env info`, so it must not hold secrets. @@ -62,6 +69,48 @@ Scopes are **not** env-var overridable; set them in the compiled-in layer or `en Bag keys in `Environment::extra` are overridable via `_` only when the key is already present in the merged record after layers 1 and 2. For example, `api_url` must exist in either the compiled defaults or the file before `PROD_API_URL` has any effect. +## Feature-Flag Layering + +Feature-flag visibility is a fourth resolution axis, parallel to but independent from the OAuth/bag-key layers above. See [Feature Flags & Stages](concepts.md#feature-flags--stages) for what `Stage` and `FlagPolicy` mean; this section covers only the environment-specific plumbing. + +`EnvironmentDef` carries `min_stage: Option` and `feature_overrides: BTreeMap`, set with `.with_min_stage(stage)` and `.with_feature_override(key, stage)`. The resolved `Environment` mirrors both fields. They merge through the same three layers as OAuth/bag fields — compiled defaults, then `environments.toml`, then environment variables — and are then layered onto the consumer's own `CliConfig`-level policy. + +In `environments.toml`, `min_stage` is a plain key on the environment's table, and per-key overrides go in a nested `[.feature_overrides]` table: + +```toml +[dev] +min_stage = "experimental" + +[staging.feature_overrides] +"domain-bulk-transfer" = "ga" +``` + +An override must itself meet the active `min_stage` floor to reveal a node — overriding a command's stage to `beta` while `min_stage` stays `ga` still hides it (visibility requires `effective_stage >= min_stage`, and `beta < ga`); override to `ga` to reveal it regardless of the node's own declared stage. That is why the `staging` example above forces `domain-bulk-transfer` to `ga`: `staging` sets no `min_stage`, so its floor is the `Ga` default, and only a `ga` override clears it — one surgical unlock of that command while every other still-gated node stays hidden. + +Environment-variable overrides: + +| Variable | Field overridden | +| --- | --- | +| `_MIN_STAGE` | `min_stage` | +| `_FEATURE_` | `feature_overrides[]` | + +`_FEATURE_` follows the same restriction as bag keys: it only takes effect when `` is already present in `feature_overrides` after the compiled+file merge (layers 1 and 2). `` is the flag key uppercased with `-` replaced by `_` (`domain-bulk-transfer` → `PROD_FEATURE_DOMAIN_BULK_TRANSFER`). + +The full precedence order, highest wins: + +```text +env var for a specific key (_FEATURE_) + > env var min-stage (_MIN_STAGE) + > environment file's feature_overrides for that key + > environment file's min_stage + > consumer .with_feature_override(...) + > consumer .with_min_stage(...) (default Stage::Ga) +``` + +The environment layer is applied only when environment resolution succeeds: if resolving the active environment errors — a malformed `_MIN_STAGE` value, an unparsable `environments.toml`, or an active-environment name unknown to every layer — the engine silently falls back to the consumer-level `CliConfig` policy alone and drops the environment-layer contribution rather than failing the run. In the intended usage pattern (the consumer ships a `Ga` default and an environment *loosens* it for dev/experimental builds) this fails **closed**: a resolution error simply leaves the stricter compiled default in force. But the reverse pattern is unsafe: if a consumer ships a *permissive* compiled `min_stage` (or feature override) and relies on an environment to *tighten* it for a public/production build, a resolution error fails **open** — the gated nodes the environment would have re-hidden are mounted under the permissive compiled policy instead. A security model that depends on an environment tightening a permissive compiled default must therefore validate that environment resolution succeeds (for example via `env info`, or by not caching a build whose active environment cannot resolve) rather than assuming resolution errors cannot occur in practice; do not rely on this crate to fail closed for that direction. + +Unlike the OAuth/bag-key layers, this resolved `FlagPolicy` is fixed for the life of the `Cli`: `Cli::new` computes it once from the environment seeded at startup (the sticky/persisted active environment, or the compiled default) and uses it immediately afterward to prune the command tree that `--help`, `--schema`, and dispatch all read from. Passing `--env ` on the command line (`apply_env_flag`) updates `middleware.env` for that invocation — which does change which environment other commands such as `env info` or an OAuth provider resolve against — but it does not recompute `flag_policy` or re-prune the already-built tree, so it has no effect on feature-flag visibility for that run, including what `flags list`/`flags info` report, since they read the same startup-fixed policy; the visible/hidden set is fixed to whichever environment was active when the process started, and `--env` cannot widen or narrow it. + ## Active Environment The active environment controls which environment is targeted when no `--env` flag is passed. diff --git a/examples/basic.rs b/examples/basic.rs index 8e92636..b831567 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -3,7 +3,7 @@ use std::process::ExitCode; use clap::Arg; use cli_engine::{ BuildInfo, Cli, CliConfig, CommandResult, CommandSpec, GroupSpec, Module, RuntimeCommandSpec, - RuntimeGroupSpec, + RuntimeGroupSpec, Stage, }; use serde_json::json; @@ -37,14 +37,27 @@ async fn main() -> ExitCode { }, ); + // Gated behind Stage::Experimental, so it is pruned from help/schema/dispatch under the + // default policy (min_stage: Stage::Ga). To see it, uncomment the .with_min_stage(...) line + // below (or add .with_feature_override("project-preview", Stage::Ga) instead). + let preview = RuntimeCommandSpec::new( + CommandSpec::new("preview", "Preview an upcoming project feature") + .with_system("projects-api") + .with_feature_flag("project-preview", Stage::Experimental) + .no_auth(true), + async |_credential, _args| Ok(CommandResult::new(json!({ "status": "coming-soon" }))), + ); + let project_module = Module::new("Platform Systems", move |_context| { RuntimeGroupSpec::new(GroupSpec::new("project", "Manage projects")) .with_command(list_projects.clone()) + .with_command(preview.clone()) }); let cli = Cli::new( CliConfig::new("example", "Example cli-engine application", "example") .with_build(BuildInfo::new(env!("CARGO_PKG_VERSION"))) + // .with_min_stage(Stage::Experimental) .with_module(project_module), ); diff --git a/src/cli.rs b/src/cli.rs index 8017b78..40c899c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -17,7 +17,7 @@ use clap::{ArgMatches, Command}; use crate::{ ActivityEmitter, Auditor, AuthProvider, Authorizer, CliCoreError, CommandMeta, CommandSpec, - GroupSpec, GuideEntry, Middleware, MiddlewareRequest, Result, RuntimeCommandSpec, + FeatureFlag, GroupSpec, GuideEntry, Middleware, MiddlewareRequest, Result, RuntimeCommandSpec, RuntimeGroupSpec, auth::commands::auth_command_group, command::{ @@ -25,6 +25,7 @@ use crate::{ leaf_matches, }, error::exit_code_for_error, + feature_flags::{FlagEntry, FlagPolicy, FlagRegistry, Stage}, flags::{ GlobalFlags, default_output_format, derive_bool_flags, derive_value_flags, extract_command_path, extract_output_format, extract_search_query, @@ -275,6 +276,29 @@ pub struct CliConfig { /// middleware, and exposes it to handlers through /// [`CommandContext::environment`](crate::command::CommandContext::environment). pub environments: Option>, + /// Minimum feature stage required for a flagged command, group, or module + /// to remain mounted. + /// + /// Defaults to [`Stage::Ga`] via [`Stage`]'s own `Default`, which combined + /// with an empty [`feature_overrides`](Self::feature_overrides) is the + /// zero-config behavior: nothing is gated unless a command/group/module + /// opts in with `.with_feature_flag(...)`, and even then it stays visible + /// until this is lowered. Lower it (e.g. to [`Stage::Beta`] or + /// [`Stage::Experimental`]) to opt a build or environment into + /// pre-release commands. Set via [`CliConfig::with_min_stage`]. + pub min_stage: Stage, + /// Per-key stage overrides that substitute a forced stage for a flag + /// key's own declared stage before comparing against + /// [`min_stage`](Self::min_stage). + /// + /// Empty by default. Populate via [`CliConfig::with_feature_override`] to + /// force one named flag to a specific effective stage — e.g. forcing a + /// single flag to [`Stage::Ga`] to turn it on for internal testing without + /// lowering [`min_stage`](Self::min_stage) for every other flagged + /// command, or forcing it to [`Stage::Experimental`] to disable it even + /// under a permissive `min_stage`. See [`FlagPolicy::visible`] for the + /// exact comparison. + pub feature_overrides: BTreeMap, } impl CliConfig { @@ -351,6 +375,37 @@ impl CliConfig { self } + /// Sets the minimum feature stage required for a flagged command, group, + /// or module to remain mounted. + /// + /// See [`min_stage`](Self::min_stage) for the default and [`FlagPolicy`] + /// for how it combines with [`feature_overrides`](Self::feature_overrides) + /// during command-tree pruning. + #[must_use] + pub fn with_min_stage(mut self, stage: Stage) -> Self { + self.min_stage = stage; + self + } + + /// Adds (or replaces) a per-key feature-flag stage override. + /// + /// See [`feature_overrides`](Self::feature_overrides) for how the + /// override participates in the [`FlagPolicy::visible`] comparison. + #[must_use] + pub fn with_feature_override(mut self, key: impl Into, stage: Stage) -> Self { + self.feature_overrides.insert(key.into(), stage); + self + } + + /// Builds the merged [`FlagPolicy`] used for command-tree pruning from + /// this config's `min_stage` and `feature_overrides`. + fn flag_policy(&self) -> FlagPolicy { + FlagPolicy { + min_stage: self.min_stage, + overrides: self.feature_overrides.clone(), + } + } + /// Overrides the outbound User-Agent string for all HTTP traffic. /// /// When unset, the engine derives `name/version` from this config (see @@ -677,6 +732,8 @@ impl std::fmt::Debug for CliConfig { "argv0_routes", &self.argv0_routes.keys().collect::>(), ) + .field("min_stage", &self.min_stage) + .field("feature_overrides", &self.feature_overrides) .finish() } } @@ -855,6 +912,27 @@ impl Cli { middleware.env = environments.effective_active(None, &middleware.config); middleware.environments = Some(Arc::clone(environments)); } + // Seed the merged flag policy before any module/group is registered so + // pruning during `add_module`/`add_module_group` below sees it. The active + // environment's fully-resolved (compiled + file + env-var) min_stage/ + // feature_overrides, when present, take precedence over the consumer-level + // CliConfig policy — an environment can loosen or tighten visibility beyond + // what the binary set in code. Environment resolution failure (e.g. an + // unknown active environment name) is tolerated here and falls back to the + // consumer-level policy only: this is just flag-policy computation, not full + // environment validation, and must not make `Cli::new` fail in a new way. + // The normal lazy paths (`env get`/`env info`, `ctx.environment()?`) still + // surface a real resolution error to the user when a command needs it. + let mut flag_policy = config.flag_policy(); + if let Some(environments) = &middleware.environments + && let Ok(env) = environments.resolve(&middleware.env) + { + if let Some(min_stage) = env.min_stage { + flag_policy.min_stage = min_stage; + } + flag_policy.overrides.extend(env.feature_overrides); + } + middleware.flag_policy = flag_policy; let mut cli = Self { config, @@ -899,6 +977,7 @@ impl Cli { if cli.config.environments.is_some() { cli.ensure_env_command(); } + cli.ensure_flags_command(); cli } @@ -1037,6 +1116,20 @@ impl Cli { &mut self, category: impl Into, group: RuntimeGroupSpec, + ) -> &mut Self { + self.add_module_group_inner(category, group, None) + } + + /// Shared implementation behind [`add_module_group`](Self::add_module_group) + /// and [`add_module`](Self::add_module). `inherited` is the effective + /// feature flag the group's enclosing module declared (if any), so a + /// module-level flag cascades down to the group even though + /// `add_module_group` itself has no concept of a module. + fn add_module_group_inner( + &mut self, + category: impl Into, + group: RuntimeGroupSpec, + inherited: Option, ) -> &mut Self { // Prevent consumer modules from shadowing engine built-ins in the clap // command tree. A reserved group name would override the engine's own @@ -1048,6 +1141,18 @@ impl Cli { ); return self; } + + let mut prefix = Vec::new(); + let Some(group) = prune_feature_flag_tree( + group, + inherited.as_ref(), + &self.middleware.flag_policy, + &mut prefix, + &mut self.middleware.flag_registry, + ) else { + return self; + }; + let category = category.into(); if !group.group.hidden { self.module_entries.push(ModuleHelpEntry { @@ -1090,7 +1195,7 @@ impl Cli { self.middleware.human_views.register(view); } self.add_guides(guides); - self.add_module_group(module.category, group) + self.add_module_group_inner(module.category, group, module.feature_flag.clone()) } /// Adds one top-level runtime command after construction. @@ -2133,6 +2238,40 @@ impl Cli { self.refresh_root_long(); } + /// Mounts the built-in `flags` command group and files it under the admin + /// help category. Idempotent and yields to a consumer-defined `flags` + /// subcommand if one already exists. Unlike [`Self::ensure_env_command`], + /// this is mounted unconditionally: feature-flag introspection does not + /// depend on any opt-in system, so it is always available. + fn ensure_flags_command(&mut self) { + if has_subcommand(&self.root, "flags") { + return; + } + let group = crate::flag_commands::flags_command_group(); + let mut prefix = Vec::new(); + group.register_commands(&mut prefix, &mut self.commands); + let mut prefix = Vec::new(); + let clap_group = runtime_group_clap_command_with_schema_help( + &group, + &mut prefix, + &self.middleware.schema_registry, + ); + self.root = self.root.clone().subcommand(clap_group); + let category = self + .config + .admin_category + .clone() + .unwrap_or_else(|| DEFAULT_ADMIN_CATEGORY.to_owned()); + if !self.module_entries.iter().any(|e| e.name == "flags") { + self.module_entries.push(ModuleHelpEntry { + category, + name: "flags".to_owned(), + short: "Inspect declared feature flags".to_owned(), + }); + } + self.refresh_root_long(); + } + fn default_auth_provider(&self) -> String { if !self.middleware.default_auth_provider.is_empty() { return self.middleware.default_auth_provider.clone(); @@ -2953,6 +3092,106 @@ fn make_executable(_path: &Path) -> std::io::Result<()> { Ok(()) } +/// Walks a runtime group tree, resolving each node's effective feature flag by +/// cascading from `inherited` — a node's own [`GroupSpec::feature_flag`] or +/// [`CommandSpec::feature_flag`] wins if set, otherwise it inherits the +/// nearest ancestor's effective flag, otherwise (nothing in the ancestor +/// chain declared a flag) it implicitly resolves to [`Stage::Ga`] with no key. +/// Every node that resolves to a *named* flag (own or inherited) is recorded +/// into `registry` under its colon-separated path, together with whether +/// `policy` judged it visible. Nodes that resolve to the implicit no-flag +/// default are not recorded (there is nothing to introspect) and are always +/// visible. +/// +/// Returns `None` when this group itself should be dropped from the tree — +/// either because its effective flag is not visible under `policy`, or +/// because every one of its commands and subgroups was pruned away, leaving +/// an empty group with nothing to mount. An emptied-out group is dropped +/// unconditionally, even if its own flag was visible: a `clap` subcommand +/// group with zero children is useless either way, so this simplifies the +/// pruning logic rather than threading through a "was this group itself +/// visible but empty" distinction that no caller needs. +/// +/// Note that an invisible ancestor short-circuits before its children are +/// even visited: a more permissive flag on a descendant cannot resurrect a +/// subtree whose enclosing group already failed the visibility check. +fn prune_feature_flag_tree( + mut group: RuntimeGroupSpec, + inherited: Option<&FeatureFlag>, + policy: &FlagPolicy, + prefix: &mut Vec, + registry: &mut FlagRegistry, +) -> Option { + prefix.push(group.group.name.clone()); + + let effective = group + .group + .feature_flag + .clone() + .or_else(|| inherited.cloned()); + if !record_and_check_visibility(effective.as_ref(), policy, prefix, registry) { + prefix.pop(); + return None; + } + + let mut kept_groups = Vec::with_capacity(group.groups.len()); + for child in std::mem::take(&mut group.groups) { + if let Some(pruned) = + prune_feature_flag_tree(child, effective.as_ref(), policy, prefix, registry) + { + kept_groups.push(pruned); + } + } + group.groups = kept_groups; + + let mut kept_commands = Vec::with_capacity(group.commands.len()); + for command in std::mem::take(&mut group.commands) { + prefix.push(command.spec.name.clone()); + let command_effective = command + .spec + .feature_flag + .clone() + .or_else(|| effective.clone()); + let visible = + record_and_check_visibility(command_effective.as_ref(), policy, prefix, registry); + prefix.pop(); + if visible { + kept_commands.push(command); + } + } + group.commands = kept_commands; + + prefix.pop(); + + if group.commands.is_empty() && group.groups.is_empty() { + None + } else { + Some(group) + } +} + +/// Records `effective` at the current `prefix` path into `registry` (only +/// when it names a flag key — the implicit Ga default is not recorded) and +/// returns whether the node is visible under `policy`. +fn record_and_check_visibility( + effective: Option<&FeatureFlag>, + policy: &FlagPolicy, + prefix: &[String], + registry: &mut FlagRegistry, +) -> bool { + let Some(flag) = effective else { + return true; + }; + let visible = policy.visible(Some(flag.key.as_str()), flag.stage); + registry.record(FlagEntry { + path: prefix.join(":"), + key: flag.key.clone(), + stage: flag.stage, + visible, + }); + visible +} + fn register_runtime_group_metadata( group: &RuntimeGroupSpec, prefix: &mut Vec, @@ -3265,3 +3504,431 @@ mod env_config_tests { assert!(out.rendered.contains("nope")); } } + +#[cfg(test)] +mod feature_flag_pruning_tests { + use super::*; + use crate::CommandResult; + + fn trivial_command(name: &str) -> RuntimeCommandSpec { + RuntimeCommandSpec::new( + CommandSpec::new(name, "short").no_auth(true), + async |_, _| Ok(CommandResult::new(serde_json::Value::Null)), + ) + } + + fn flagged_command(name: &str, key: &str, stage: Stage) -> RuntimeCommandSpec { + let mut command = trivial_command(name); + command.spec = command.spec.with_feature_flag(key, stage); + command + } + + fn empty_policy() -> FlagPolicy { + FlagPolicy::default() + } + + #[test] + fn no_flags_anywhere_keeps_everything() { + let group = RuntimeGroupSpec::new(GroupSpec::new("root", "short")) + .with_command(trivial_command("a")) + .with_command(trivial_command("b")) + .with_group( + RuntimeGroupSpec::new(GroupSpec::new("child", "short")) + .with_command(trivial_command("c")), + ); + + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = + prune_feature_flag_tree(group, None, &empty_policy(), &mut prefix, &mut registry); + + let pruned = pruned.expect("unflagged tree should never be dropped"); + assert_eq!(pruned.commands.len(), 2); + assert_eq!(pruned.groups.len(), 1); + assert_eq!(pruned.groups[0].commands.len(), 1); + assert!(registry.entries().is_empty()); + } + + #[test] + fn experimental_command_is_pruned_sibling_is_not() { + let group = RuntimeGroupSpec::new(GroupSpec::new("root", "short")) + .with_command(flagged_command("gated", "gated-flag", Stage::Experimental)) + .with_command(trivial_command("sibling")); + + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = + prune_feature_flag_tree(group, None, &empty_policy(), &mut prefix, &mut registry) + .expect("group still has a visible command left"); + + assert_eq!(pruned.commands.len(), 1); + assert_eq!(pruned.commands[0].spec.name, "sibling"); + + let entries = registry.entries(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].path, "root:gated"); + assert_eq!(entries[0].key, "gated-flag"); + assert!(!entries[0].visible); + } + + #[test] + fn beta_group_pruned_under_ga_min_stage_kept_under_beta_min_stage() { + let build_tree = || { + RuntimeGroupSpec::new(GroupSpec::new("root", "short")) + .with_command(trivial_command("keep-me")) + .with_group( + RuntimeGroupSpec::new( + GroupSpec::new("flagged-group", "short") + .with_feature_flag("group-flag", Stage::Beta), + ) + .with_command(trivial_command("cmd-default")) + .with_command(flagged_command( + "cmd-ga", + "cmd-ga-flag", + Stage::Ga, + )), + ) + }; + + // Default policy (min_stage: Ga) drops the whole Beta subtree, including + // both its undeclared and explicitly-Ga-declared children, because the + // ancestor group itself already fails visibility before children are + // even visited. + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = prune_feature_flag_tree( + build_tree(), + None, + &empty_policy(), + &mut prefix, + &mut registry, + ) + .expect("root keeps its unflagged sibling command"); + assert!(pruned.groups.is_empty()); + assert_eq!(pruned.commands.len(), 1); + assert_eq!(pruned.commands[0].spec.name, "keep-me"); + // Only the group itself was recorded; its children were never visited. + assert_eq!(registry.entries().len(), 1); + assert_eq!(registry.entries()[0].path, "root:flagged-group"); + assert!(!registry.entries()[0].visible); + + // A Beta-permissive policy keeps the group and both of its children. + let policy = FlagPolicy::default().with_min_stage(Stage::Beta); + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = + prune_feature_flag_tree(build_tree(), None, &policy, &mut prefix, &mut registry) + .expect("root is kept"); + assert_eq!(pruned.groups.len(), 1); + assert_eq!(pruned.groups[0].commands.len(), 2); + assert!(registry.entries().iter().all(|entry| entry.visible)); + } + + #[test] + fn ancestor_invisibility_short_circuits_before_children_are_visited() { + // The child declares its own, more permissive Ga flag under a distinct + // key. Per the documented pruning semantics, an invisible ancestor drops + // its whole subtree unconditionally: the child's own flag is never even + // considered, because `prune_feature_flag_tree` returns `None` for the + // ancestor as soon as its own effective flag fails visibility, before + // recursing into commands or subgroups at all. + let group = RuntimeGroupSpec::new( + GroupSpec::new("ancestor", "short").with_feature_flag("ancestor-flag", Stage::Beta), + ) + .with_command(flagged_command("child", "child-flag", Stage::Ga)); + + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = + prune_feature_flag_tree(group, None, &empty_policy(), &mut prefix, &mut registry); + + assert!( + pruned.is_none(), + "invisible ancestor drops its whole subtree" + ); + // The child was never visited, so nothing about it was recorded. + assert_eq!(registry.entries().len(), 1); + assert_eq!(registry.entries()[0].path, "ancestor"); + assert!(registry.by_key("child-flag").is_empty()); + } + + #[test] + fn cascading_inherited_flag_key_and_stage_reach_unflagged_descendants() { + // Simulates a module-level flag with no per-group/per-command + // declaration anywhere below it: `inherited` here stands in for + // `Module::feature_flag`, exactly as `add_module_group_inner` passes it. + let module_flag = FeatureFlag::new("module-flag", Stage::Beta); + let group = RuntimeGroupSpec::new(GroupSpec::new("root", "short")) + .with_command(trivial_command("unflagged-child")); + + let policy = FlagPolicy::default().with_min_stage(Stage::Beta); + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = prune_feature_flag_tree( + group, + Some(&module_flag), + &policy, + &mut prefix, + &mut registry, + ) + .expect("Beta-permissive policy keeps a Beta-inherited tree"); + assert_eq!(pruned.commands.len(), 1); + + // Both the group and the descendant command recorded the *same* + // inherited key/stage, proving real cascading rather than an implicit + // Ga default at either level. + let entries = registry.entries(); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0].path, "root"); + assert_eq!(entries[0].key, "module-flag"); + assert_eq!(entries[0].stage, Stage::Beta); + assert_eq!(entries[1].path, "root:unflagged-child"); + assert_eq!(entries[1].key, "module-flag"); + assert_eq!(entries[1].stage, Stage::Beta); + + // Under the default (Ga) policy the same inherited Beta flag makes the + // whole tree invisible together, since the group and its unflagged + // child resolve to the identical effective flag. + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = prune_feature_flag_tree( + RuntimeGroupSpec::new(GroupSpec::new("root", "short")) + .with_command(trivial_command("unflagged-child")), + Some(&module_flag), + &empty_policy(), + &mut prefix, + &mut registry, + ); + assert!(pruned.is_none()); + } + + #[test] + fn registry_records_only_named_flags_not_unflagged_nodes() { + let group = RuntimeGroupSpec::new(GroupSpec::new("root", "short")).with_group( + RuntimeGroupSpec::new( + GroupSpec::new("g", "short").with_feature_flag("g-flag", Stage::Beta), + ) + .with_command(trivial_command("c1")) + .with_command(flagged_command("c2", "c2-flag", Stage::Ga)), + ); + + // Permissive enough that nothing is pruned, so every node is visited. + let policy = FlagPolicy::default().with_min_stage(Stage::Experimental); + let mut prefix = Vec::new(); + let mut registry = FlagRegistry::new(); + let pruned = prune_feature_flag_tree(group, None, &policy, &mut prefix, &mut registry) + .expect("permissive policy keeps everything"); + assert_eq!(pruned.groups[0].commands.len(), 2); + + let entries = registry.entries(); + assert_eq!(entries.len(), 3, "root has no flag and is not recorded"); + assert_eq!(entries[0].path, "root:g"); + assert_eq!(entries[0].key, "g-flag"); + assert_eq!(entries[1].path, "root:g:c1"); + assert_eq!(entries[1].key, "g-flag"); + assert_eq!(entries[1].stage, Stage::Beta); + assert_eq!(entries[2].path, "root:g:c2"); + assert_eq!(entries[2].key, "c2-flag"); + assert_eq!(entries[2].stage, Stage::Ga); + assert!(entries.iter().all(|entry| entry.visible)); + } + + #[test] + fn module_feature_flag_cascades_into_its_group_via_add_module() { + // Regression test for the bug this task fixes: `add_module` used to + // discard `module.feature_flag` entirely, so a module-level flag could + // never reach its group/commands. `Module::new` returns a group with an + // unflagged command; the module itself declares Experimental, and the + // default (Ga) policy must prune the whole group away. + let module = Module::new("Test Category", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("gated-mod", "short")) + .with_command(trivial_command("list")) + }) + .with_feature_flag("module-flag", Stage::Experimental); + + let mut cli = Cli::new(CliConfig::new("modtest", "Module test", "modtest")); + cli.add_module(module); + + assert!( + !cli.commands.contains_key("gated-mod:list"), + "module-level Experimental flag should have pruned the whole group under the default Ga policy" + ); + assert!( + !has_subcommand(&cli.root, "gated-mod"), + "the pruned group must not be mounted in the clap tree either" + ); + } + + #[test] + fn module_feature_flag_keeps_group_when_policy_allows_it() { + let module = Module::new("Test Category", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("gated-mod-2", "short")) + .with_command(trivial_command("list")) + }) + .with_feature_flag("module-flag-2", Stage::Experimental); + + let mut cli = Cli::new( + CliConfig::new("modtest2", "Module test", "modtest2") + .with_min_stage(Stage::Experimental), + ); + cli.add_module(module); + + assert!(cli.commands.contains_key("gated-mod-2:list")); + assert!(has_subcommand(&cli.root, "gated-mod-2")); + } + + #[test] + fn active_environment_min_stage_loosens_consumer_level_policy() { + // The CliConfig itself leaves min_stage at its Ga default, which would + // normally prune this Experimental-flagged group. The active ("prod") + // environment's compiled min_stage override should reach + // `middleware.flag_policy` before pruning runs and keep it instead. + let module = Module::new("Test Category", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("gated-mod-3", "short")) + .with_command(trivial_command("list")) + }) + .with_feature_flag("module-flag-3", Stage::Experimental); + + let mut cli = Cli::new( + CliConfig::new("modtest3", "Module test", "modtest3").with_environments(Arc::new( + crate::environments::Environments::new("prod").with_environment( + "prod", + crate::environments::EnvironmentDef::new().with_min_stage(Stage::Experimental), + ), + )), + ); + cli.add_module(module); + + assert!(cli.commands.contains_key("gated-mod-3:list")); + assert!(has_subcommand(&cli.root, "gated-mod-3")); + } +} + +#[cfg(test)] +mod flags_command_tests { + use super::*; + use crate::CommandResult; + + /// Builds a module with one flagged group containing one flagged (via + /// inheritance) `list` command, so `flag_registry` has something to + /// introspect once the module is mounted. + fn flagged_module(group_name: &'static str, key: &'static str, stage: Stage) -> Module { + Module::new("Test Category", move |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new(group_name, "short")).with_command( + RuntimeCommandSpec::new( + CommandSpec::new("list", "short").no_auth(true), + async |_, _| Ok(CommandResult::new(serde_json::Value::Null)), + ), + ) + }) + .with_feature_flag(key, stage) + } + + #[tokio::test] + async fn flags_list_reports_flagged_entries() { + let mut cli = Cli::new( + CliConfig::new("flagtest", "Flag test", "flagtest").with_min_stage(Stage::Beta), + ); + cli.add_module(flagged_module("flagged-mod", "list-flag", Stage::Beta)); + + let out = cli + .run(["flagtest", "flags", "list", "--output", "json"]) + .await; + assert_eq!(out.exit_code, 0, "rendered: {}", out.rendered); + let rendered: serde_json::Value = + serde_json::from_str(&out.rendered).expect("stdout should contain json"); + let entries = rendered["data"].as_array().expect("data should be array"); + let command_entry = entries + .iter() + .find(|entry| entry["path"] == "flagged-mod:list") + .expect("flagged command entry should be present"); + assert_eq!(command_entry["key"], "list-flag"); + assert_eq!(command_entry["stage"], "beta"); + assert_eq!(command_entry["visible"], true); + } + + #[tokio::test] + async fn flags_info_returns_policy_and_entries_for_known_key() { + let mut cli = Cli::new( + CliConfig::new("flagtest2", "Flag test", "flagtest2").with_min_stage(Stage::Beta), + ); + cli.add_module(flagged_module("flagged-mod-2", "info-flag", Stage::Beta)); + + let out = cli + .run([ + "flagtest2", + "flags", + "info", + "info-flag", + "--output", + "json", + ]) + .await; + assert_eq!(out.exit_code, 0, "rendered: {}", out.rendered); + let rendered: serde_json::Value = + serde_json::from_str(&out.rendered).expect("stdout should contain json"); + let data = &rendered["data"]; + assert_eq!(data["key"], "info-flag"); + assert_eq!(data["policy"]["min_stage"], "beta"); + assert!(data["policy"]["override"].is_null()); + let entries = data["entries"].as_array().expect("entries should be array"); + assert!(!entries.is_empty()); + assert!(entries.iter().any(|entry| { + entry["path"] == "flagged-mod-2:list" && entry["decided_by"] == "min_stage" + })); + } + + #[tokio::test] + async fn flags_info_reports_override_decided_by() { + // The module declares Experimental, which the default Ga policy would + // normally hide; the override forces Ga instead, so the entries stay + // visible even though `entry.stage` still reports the node's own + // (Experimental) declaration, not the override. + let mut cli = Cli::new( + CliConfig::new("flagtest3", "Flag test", "flagtest3") + .with_feature_override("override-flag", Stage::Ga), + ); + cli.add_module(flagged_module( + "flagged-mod-3", + "override-flag", + Stage::Experimental, + )); + + let out = cli + .run([ + "flagtest3", + "flags", + "info", + "override-flag", + "--output", + "json", + ]) + .await; + assert_eq!(out.exit_code, 0, "rendered: {}", out.rendered); + let rendered: serde_json::Value = + serde_json::from_str(&out.rendered).expect("stdout should contain json"); + let data = &rendered["data"]; + assert_eq!(data["policy"]["min_stage"], "ga"); + assert_eq!(data["policy"]["override"], "ga"); + let entries = data["entries"].as_array().expect("entries should be array"); + assert!(!entries.is_empty()); + assert!( + entries + .iter() + .all(|entry| entry["decided_by"] == "override") + ); + assert!(entries.iter().all(|entry| entry["visible"] == true)); + assert!(entries.iter().all(|entry| entry["stage"] == "experimental")); + } + + #[tokio::test] + async fn flags_info_unknown_key_errors() { + let cli = Cli::new(CliConfig::new("flagtest4", "Flag test", "flagtest4")); + + let out = cli + .run(["flagtest4", "flags", "info", "no-such-flag"]) + .await; + assert_ne!(out.exit_code, 0); + assert!(out.rendered.contains("no such flag")); + } +} diff --git a/src/command.rs b/src/command.rs index d9e3047..81c28e9 100644 --- a/src/command.rs +++ b/src/command.rs @@ -6,8 +6,8 @@ use serde_json::{Number, Value}; use tokio::sync::mpsc; use crate::{ - AuthRequirement, CommandMeta, Credential, CredentialResolver, Middleware, OutputSchema, Result, - SchemaInfo, Tier, + AuthRequirement, CommandMeta, Credential, CredentialResolver, FeatureFlag, Middleware, + OutputSchema, Result, SchemaInfo, Stage, Tier, middleware::ValueMap, output::{NextAction, TableColumn}, }; @@ -272,6 +272,19 @@ pub struct CommandSpec { /// module or CLI, so several commands can share one table. Takes precedence /// over inline [`view_columns`](CommandSpec::view_columns). pub view_id: Option, + /// This command's own feature-flag declaration, if any. + /// + /// `None` means the command has no explicit stage declaration of its own, + /// in which case it inherits its effective stage from its nearest ancestor + /// (nested group, then enclosing group, then module — nearest declaration + /// wins), implicitly resolving to [`Stage::Ga`] if nothing in the ancestor + /// chain declares a flag either; see [`Stage`]'s documentation for why + /// that is its default. Set with + /// [`with_feature_flag`](CommandSpec::with_feature_flag). This field only + /// records the command's own declaration; cascading resolution against the + /// ancestor chain happens when a [`Cli`](crate::Cli) mounts the enclosing + /// module or group. + pub feature_flag: Option, } impl CommandSpec { @@ -416,6 +429,14 @@ impl CommandSpec { self } + /// Declares this command's own feature flag: the key used for policy + /// overrides and introspection, and the stage at which it becomes visible. + #[must_use] + pub fn with_feature_flag(mut self, key: impl Into, stage: Stage) -> Self { + self.feature_flag = Some(FeatureFlag::new(key, stage)); + self + } + /// Adds provider-specific auth metadata. #[must_use] pub fn with_auth_metadata(mut self, key: impl Into, value: impl Into) -> Self { @@ -558,6 +579,18 @@ pub struct GroupSpec { pub commands: Vec, /// Declarative nested groups used for static tree construction. pub groups: Vec, + /// This group's own feature-flag declaration, if any. + /// + /// `None` means the group has no explicit stage declaration of its own, in + /// which case it inherits its effective stage from its nearest ancestor + /// (enclosing group, then module — nearest declaration wins), implicitly + /// resolving to [`Stage::Ga`] if nothing in the ancestor chain declares a + /// flag either; see [`Stage`]'s documentation for why that is its default. + /// Set with [`with_feature_flag`](GroupSpec::with_feature_flag). This field + /// only records the group's own declaration; cascading resolution against + /// the ancestor chain happens when a [`Cli`](crate::Cli) mounts the + /// enclosing module or parent group. + pub feature_flag: Option, } impl GroupSpec { @@ -606,6 +639,14 @@ impl GroupSpec { self } + /// Declares this group's own feature flag: the key used for policy overrides + /// and introspection, and the stage at which it becomes visible. + #[must_use] + pub fn with_feature_flag(mut self, key: impl Into, stage: Stage) -> Self { + self.feature_flag = Some(FeatureFlag::new(key, stage)); + self + } + /// Builds the `clap` command for parser registration. #[must_use] pub fn clap_command(&self) -> Command { @@ -990,3 +1031,48 @@ where _ => Some(Value::Array(values)), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn command_spec_with_feature_flag_sets_key_and_stage() { + let spec = + CommandSpec::new("list", "List things").with_feature_flag("my-flag", Stage::Beta); + + let flag = spec + .feature_flag + .as_ref() + .expect("feature flag should be set"); + assert_eq!(flag.key, "my-flag"); + assert_eq!(flag.stage, Stage::Beta); + } + + #[test] + fn command_spec_feature_flag_defaults_to_none() { + let spec = CommandSpec::new("list", "List things"); + + assert!(spec.feature_flag.is_none()); + } + + #[test] + fn group_spec_with_feature_flag_sets_key_and_stage() { + let group = GroupSpec::new("project", "Manage projects") + .with_feature_flag("my-flag", Stage::Experimental); + + let flag = group + .feature_flag + .as_ref() + .expect("feature flag should be set"); + assert_eq!(flag.key, "my-flag"); + assert_eq!(flag.stage, Stage::Experimental); + } + + #[test] + fn group_spec_feature_flag_defaults_to_none() { + let group = GroupSpec::new("project", "Manage projects"); + + assert!(group.feature_flag.is_none()); + } +} diff --git a/src/environments.rs b/src/environments.rs index 72b3084..25a254e 100644 --- a/src/environments.rs +++ b/src/environments.rs @@ -8,7 +8,7 @@ use std::collections::BTreeMap; use serde::Deserialize; -use crate::{Result, error::CliCoreError}; +use crate::{Result, Stage, error::CliCoreError}; /// Standard OAuth slice of an environment, consumed by `PkceAuthProvider`. /// @@ -38,6 +38,11 @@ pub struct Environment { pub oauth: Option, /// App-specific fields (for example `api_url`). pub extra: BTreeMap, + /// Bulk override for this environment's minimum visible feature stage, if set + /// by any layer (compiled, file, or env var). + pub min_stage: Option, + /// Per-key feature-stage overrides declared for this environment. + pub feature_overrides: BTreeMap, } /// An unresolved per-environment declaration (one layer of configuration). @@ -54,6 +59,10 @@ pub struct EnvironmentDef { token_url: Option, #[serde(default)] scopes: Option>, + #[serde(default)] + min_stage: Option, + #[serde(default)] + feature_overrides: BTreeMap, /// Everything not recognised above is captured here (app-specific fields). #[serde(flatten, default)] extra: BTreeMap, @@ -100,6 +109,20 @@ impl EnvironmentDef { self.extra.insert(key.into(), value.into()); self } + + /// Sets a bulk override for this environment's minimum visible feature stage. + #[must_use] + pub fn with_min_stage(mut self, stage: Stage) -> Self { + self.min_stage = Some(stage); + self + } + + /// Sets a per-key feature-stage override for this environment. + #[must_use] + pub fn with_feature_override(mut self, key: impl Into, stage: Stage) -> Self { + self.feature_overrides.insert(key.into(), stage); + self + } } /// Engine-owned environment system: definitions + resolution + active-env state. @@ -236,7 +259,7 @@ impl Environments { if let Some(def) = &file { merge_into(&mut merged, def); } - apply_env_vars(name, &mut merged); + apply_env_vars(name, &mut merged)?; Ok(finalize(name, merged)) } @@ -338,20 +361,34 @@ fn merge_into(dst: &mut EnvironmentDef, src: &EnvironmentDef) { if src.scopes.is_some() { dst.scopes = src.scopes.clone(); } + if src.min_stage.is_some() { + dst.min_stage = src.min_stage; + } + for (k, v) in &src.feature_overrides { + dst.feature_overrides.insert(k.clone(), *v); + } for (k, v) in &src.extra { dst.extra.insert(k.clone(), v.clone()); } } -/// Applies `_*` overrides: the three OAuth fields always, and any bag key -/// already present in the merged record (keyed `_`). +/// Applies `_*` overrides: the three OAuth fields always, any bag key +/// already present in the merged record (keyed `_`), the bulk +/// `_MIN_STAGE` stage override, and any feature key already present in +/// the merged record (keyed `_FEATURE_`). /// /// The prefix is `name.to_uppercase().replace('-', "_")`, so environment names /// that differ only by `-` vs `_` map to the same prefix and will collide. /// /// Scopes are intentionally not env-var overridable; set them via the /// compiled-in layer or the `environments.toml` file. -fn apply_env_vars(name: &str, def: &mut EnvironmentDef) { +/// +/// # Errors +/// +/// Returns an error when `_MIN_STAGE` or `_FEATURE_` is set but +/// fails to parse as a [`Stage`]. Unlike a missing var (a no-op), a malformed +/// value is not silently ignored. +fn apply_env_vars(name: &str, def: &mut EnvironmentDef) -> Result<()> { let prefix = name.to_uppercase().replace('-', "_"); if let Ok(v) = std::env::var(format!("{prefix}_OAUTH_CLIENT_ID")) { def.client_id = Some(v); @@ -369,6 +406,22 @@ fn apply_env_vars(name: &str, def: &mut EnvironmentDef) { def.extra.insert(key, v); } } + if let Ok(v) = std::env::var(format!("{prefix}_MIN_STAGE")) { + def.min_stage = Some(v.parse::().map_err(|err| { + CliCoreError::message(format!("invalid {prefix}_MIN_STAGE {v:?}: {err}")) + })?); + } + let feature_keys: Vec = def.feature_overrides.keys().cloned().collect(); + for key in feature_keys { + let var = format!("{prefix}_FEATURE_{}", key.to_uppercase().replace('-', "_")); + if let Ok(v) = std::env::var(&var) { + let stage = v + .parse::() + .map_err(|err| CliCoreError::message(format!("invalid {var} {v:?}: {err}")))?; + def.feature_overrides.insert(key, stage); + } + } + Ok(()) } /// Turns a fully-merged declaration into a resolved [`Environment`]. OAuth is @@ -379,6 +432,8 @@ fn finalize(name: &str, def: EnvironmentDef) -> Environment { auth_url, token_url, scopes, + min_stage, + feature_overrides, extra, } = def; let oauth = client_id.map(|id| OAuthConfig { @@ -391,6 +446,8 @@ fn finalize(name: &str, def: EnvironmentDef) -> Environment { name: name.to_owned(), oauth, extra, + min_stage, + feature_overrides, } } @@ -539,6 +596,114 @@ mod tests { ); } + #[test] + fn environment_def_min_stage_and_feature_override_builders_set_fields() { + let def = EnvironmentDef::new() + .with_min_stage(Stage::Experimental) + .with_feature_override("domain-bulk-transfer", Stage::Beta); + assert_eq!(def.min_stage, Some(Stage::Experimental)); + assert_eq!( + def.feature_overrides.get("domain-bulk-transfer"), + Some(&Stage::Beta) + ); + } + + #[test] + fn resolve_returns_compiled_min_stage_and_feature_overrides() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let envs = Environments::new("dev").with_environment( + "dev", + EnvironmentDef::new() + .with_client_id("dev-client") + .with_min_stage(Stage::Experimental) + .with_feature_override("domain-bulk-transfer", Stage::Beta), + ); + let env = envs.resolve("dev").expect("dev resolves"); + assert_eq!(env.min_stage, Some(Stage::Experimental)); + assert_eq!( + env.feature_overrides.get("domain-bulk-transfer"), + Some(&Stage::Beta) + ); + } + + #[test] + fn env_var_min_stage_overrides_compiled_and_file_layers() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // Local fixture with a compiled `min_stage` (Experimental) that differs + // from the env-var value (Beta), so a passing assertion proves the env + // var *wins over* an already-set compiled value — not merely that it + // populates an otherwise-empty field (which `sample()`'s prod, with no + // compiled `min_stage`, could not distinguish). + let envs = Environments::new("prod").with_environment( + "prod", + EnvironmentDef::new() + .with_client_id("prod-client") + .with_min_stage(Stage::Experimental), + ); + // SAFETY: serialized by ENV_LOCK; guard removes the var on any exit incl. panic. + unsafe { std::env::set_var("PROD_MIN_STAGE", "beta") }; + let _guard = EnvGuard("PROD_MIN_STAGE"); + + let env = envs.resolve("prod").expect("prod resolves"); + assert_eq!(env.min_stage, Some(Stage::Beta)); + } + + #[test] + fn env_var_feature_override_updates_existing_key() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let envs = Environments::new("prod").with_environment( + "prod", + EnvironmentDef::new().with_feature_override("domain-bulk-transfer", Stage::Beta), + ); + // SAFETY: serialized by ENV_LOCK; guard removes the var on any exit incl. panic. + unsafe { std::env::set_var("PROD_FEATURE_DOMAIN_BULK_TRANSFER", "ga") }; + let _guard = EnvGuard("PROD_FEATURE_DOMAIN_BULK_TRANSFER"); + + let env = envs.resolve("prod").expect("prod resolves"); + assert_eq!( + env.feature_overrides.get("domain-bulk-transfer"), + Some(&Stage::Ga) + ); + } + + #[test] + fn env_var_feature_override_for_undeclared_key_is_ignored() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // SAFETY: serialized by ENV_LOCK; guard removes the var on any exit incl. panic. + unsafe { std::env::set_var("PROD_FEATURE_NEVER_DECLARED", "beta") }; + let _guard = EnvGuard("PROD_FEATURE_NEVER_DECLARED"); + + let env = sample().resolve("prod").expect("prod resolves"); + assert!( + !env.feature_overrides.contains_key("never-declared"), + "an env var must not introduce a brand-new feature key" + ); + } + + #[test] + fn malformed_min_stage_env_var_errors_clearly() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // SAFETY: serialized by ENV_LOCK; guard removes the var on any exit incl. panic. + unsafe { std::env::set_var("PROD_MIN_STAGE", "nightly") }; + let _guard = EnvGuard("PROD_MIN_STAGE"); + + let err = sample().resolve("prod").unwrap_err().to_string(); + assert!( + err.contains("PROD_MIN_STAGE") && err.contains("nightly"), + "expected error to mention the var name and bad value, got: {err}" + ); + } + #[test] fn environments_file_path_sits_next_to_config() { let envs = sample().with_app_id("gddy").with_config_file(true); @@ -584,6 +749,42 @@ api_url = "https://api.custom.example.com" assert!(envs.list().contains(&"custom".to_owned())); } + #[test] + fn file_layer_min_stage_and_features_table_merge_into_feature_overrides() { + let _g = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let dir = tempfile::tempdir().expect("tempdir"); + let file = dir.path().join("environments.toml"); + std::fs::write( + &file, + r#" +[dev] +min_stage = "experimental" + +[staging] +client_id = "staging-client" + +[staging.feature_overrides] +"domain-bulk-transfer" = "beta" +"#, + ) + .expect("write file"); + + let envs = Environments::new("prod") + .with_config_file(true) + .with_config_file_path_override(file); + + let dev = envs.resolve("dev").expect("dev"); + assert_eq!(dev.min_stage, Some(Stage::Experimental)); + + let staging = envs.resolve("staging").expect("staging"); + assert_eq!( + staging.feature_overrides.get("domain-bulk-transfer"), + Some(&Stage::Beta) + ); + } + const ACTIVE_KEY: &str = "environment.active"; #[test] diff --git a/src/feature_flags.rs b/src/feature_flags.rs new file mode 100644 index 0000000..499d48b --- /dev/null +++ b/src/feature_flags.rs @@ -0,0 +1,347 @@ +//! Stage-based feature flagging primitives. +//! +//! These types describe *readiness gating*: a command, group, or module can declare +//! the [`Stage`] at which it becomes visible, and a run-wide [`FlagPolicy`] decides +//! whether that stage (or an override for a specific flag key) is currently enabled. + +use std::{collections::BTreeMap, fmt, str::FromStr}; + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +/// Feature readiness stage, used to gate commands/groups/modules before they are +/// fully promoted to general availability. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, JsonSchema, +)] +#[serde(rename_all = "lowercase")] +pub enum Stage { + /// Early, unstable functionality; visible only when explicitly opted in. + Experimental, + /// Functionally complete but still gathering feedback before general availability. + Beta, + /// Fully promoted and visible by default. + Ga, +} + +impl Stage { + /// Returns the wire string for the stage. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::Experimental => "experimental", + Self::Beta => "beta", + Self::Ga => "ga", + } + } +} + +impl Default for Stage { + /// Every command with no explicit stage declaration is implicitly [`Stage::Ga`]. + fn default() -> Self { + Self::Ga + } +} + +impl fmt::Display for Stage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for Stage { + type Err = ParseStageError; + + fn from_str(value: &str) -> Result { + match value { + "experimental" => Ok(Self::Experimental), + "beta" => Ok(Self::Beta), + "ga" => Ok(Self::Ga), + other => Err(ParseStageError { + value: other.to_owned(), + }), + } + } +} + +/// Error returned when parsing an unknown feature stage. +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] +#[error("invalid stage {value:?}: must be one of experimental, beta, ga")] +pub struct ParseStageError { + value: String, +} + +/// A named feature flag: a key (used for policy overrides and introspection) paired +/// with the stage at which the flagged node becomes visible. +#[derive(Debug, Clone)] +pub struct FeatureFlag { + /// Stable identifier used for policy overrides and introspection. + pub key: String, + /// Stage at which the flagged node becomes visible. + pub stage: Stage, +} + +impl FeatureFlag { + /// Creates a new feature flag with the given key and stage. + #[must_use] + pub fn new(key: impl Into, stage: Stage) -> Self { + Self { + key: key.into(), + stage, + } + } +} + +/// The fully-merged decision inputs for one CLI run: the minimum stage required for +/// a node to be visible, plus any per-key overrides that force a specific effective +/// stage regardless of the node's declared stage. +#[derive(Debug, Clone)] +pub struct FlagPolicy { + /// Minimum stage a node must meet (or exceed) to be visible. + pub min_stage: Stage, + /// Per-key overrides that substitute a forced effective stage for a flag key, + /// in place of the node's own declared stage, when checking visibility. + pub overrides: BTreeMap, +} + +impl Default for FlagPolicy { + fn default() -> Self { + Self { + min_stage: Stage::Ga, + overrides: BTreeMap::new(), + } + } +} + +impl FlagPolicy { + /// Creates a new policy with the default minimum stage ([`Stage::Ga`]) and no + /// overrides. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Sets the minimum stage required for a node to be visible. + #[must_use] + pub fn with_min_stage(mut self, stage: Stage) -> Self { + self.min_stage = stage; + self + } + + /// Adds (or replaces) a per-key override that forces an effective stage for the + /// given flag key, regardless of the node's own declared stage. + #[must_use] + pub fn with_override(mut self, key: impl Into, stage: Stage) -> Self { + self.overrides.insert(key.into(), stage); + self + } + + /// Returns whether a node is visible under this policy. + /// + /// If `key` is `Some` and an override is registered for it, the override's stage + /// substitutes for `stage` in the comparison against [`Self::min_stage`]. + /// Otherwise, the node's own `stage` is compared directly against + /// [`Self::min_stage`]. + #[must_use] + pub fn visible(&self, key: Option<&str>, stage: Stage) -> bool { + let effective = key + .and_then(|key| self.overrides.get(key)) + .copied() + .unwrap_or(stage); + effective >= self.min_stage + } +} + +/// One flagged node discovered while pruning a command tree. +/// +/// `path` is the colon-separated command path of the node (module/group/ +/// command name chain), matching the same convention used elsewhere in this +/// crate for command paths — e.g. a `list` command nested under a `project` +/// group records `"project:list"`. `key` and `stage` are the flag that +/// resolved for this node (its own declaration, or the nearest ancestor's, +/// per cascading resolution). `visible` is whether the policy that produced +/// this entry judged the node visible. +/// +/// Only nodes that resolve to a *named* flag are recorded; a node with no +/// flag anywhere in its ancestor chain implicitly resolves to [`Stage::Ga`] +/// with no key and is not recorded (nothing to introspect). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FlagEntry { + /// Colon-separated command path of the flagged node. + pub path: String, + /// Flag key that resolved for this node (own declaration or inherited). + pub key: String, + /// Stage the resolved flag key declared. + pub stage: Stage, + /// Whether the node was judged visible under the policy that produced it. + pub visible: bool, +} + +/// Every flagged module/group/command path discovered while pruning a +/// command tree, in registration order. +/// +/// Populated once, when a [`Cli`](crate::Cli) mounts a module or group and +/// resolves cascading feature flags across its tree. Powers `flags +/// list`/`flags info` introspection; stored on [`Middleware`](crate::Middleware) +/// and populated as a side effect of pruning. +#[derive(Debug, Clone, Default)] +pub struct FlagRegistry { + entries: Vec, +} + +impl FlagRegistry { + /// Creates an empty registry. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Records one flagged node. + pub fn record(&mut self, entry: FlagEntry) { + self.entries.push(entry); + } + + /// Returns every recorded entry, in the order they were recorded. + #[must_use] + pub fn entries(&self) -> &[FlagEntry] { + &self.entries + } + + /// Returns every recorded entry whose flag key matches `key`. + #[must_use] + pub fn by_key(&self, key: &str) -> Vec<&FlagEntry> { + self.entries + .iter() + .filter(|entry| entry.key == key) + .collect() + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + #[test] + fn stage_ordering() { + assert!(Stage::Experimental < Stage::Beta); + assert!(Stage::Beta < Stage::Ga); + assert!(Stage::Experimental < Stage::Ga); + } + + #[test] + fn stage_default_is_ga() { + assert_eq!(Stage::default(), Stage::Ga); + } + + #[test] + fn stage_from_str_round_trips() { + assert_eq!( + "experimental".parse::().unwrap(), + Stage::Experimental + ); + assert_eq!("beta".parse::().unwrap(), Stage::Beta); + assert_eq!("ga".parse::().unwrap(), Stage::Ga); + } + + #[test] + fn stage_from_str_rejects_unknown() { + let err = "nightly".parse::().unwrap_err(); + assert_eq!( + err, + ParseStageError { + value: "nightly".to_owned(), + } + ); + } + + #[test] + fn flag_policy_default_is_ga_with_no_overrides() { + let policy = FlagPolicy::default(); + assert_eq!(policy.min_stage, Stage::Ga); + assert!(policy.overrides.is_empty()); + assert!(!policy.visible(None, Stage::Beta)); + assert!(policy.visible(None, Stage::Ga)); + } + + #[test] + fn flag_policy_override_precedence() { + let policy = FlagPolicy::new() + .with_min_stage(Stage::Ga) + .with_override("my-flag", Stage::Beta); + // Override stage (Beta) is compared against min_stage (Ga), not the node's + // own declared stage (Experimental). + assert!(!policy.visible(Some("my-flag"), Stage::Experimental)); + + let policy = FlagPolicy::new() + .with_min_stage(Stage::Beta) + .with_override("my-flag", Stage::Beta); + assert!(policy.visible(Some("my-flag"), Stage::Experimental)); + } + + #[test] + fn flag_policy_no_override_falls_back_to_node_stage() { + let policy = FlagPolicy::new().with_min_stage(Stage::Beta); + assert!(!policy.visible(Some("other-flag"), Stage::Experimental)); + assert!(policy.visible(Some("other-flag"), Stage::Beta)); + assert!(policy.visible(None, Stage::Ga)); + } + + #[test] + fn flag_registry_starts_empty() { + let registry = FlagRegistry::new(); + assert!(registry.entries().is_empty()); + assert!(registry.by_key("anything").is_empty()); + } + + #[test] + fn flag_registry_records_entries_in_order() { + let mut registry = FlagRegistry::new(); + registry.record(FlagEntry { + path: "project".to_owned(), + key: "flag-a".to_owned(), + stage: Stage::Beta, + visible: true, + }); + registry.record(FlagEntry { + path: "project:list".to_owned(), + key: "flag-b".to_owned(), + stage: Stage::Experimental, + visible: false, + }); + + let entries = registry.entries(); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0].path, "project"); + assert_eq!(entries[1].path, "project:list"); + } + + #[test] + fn flag_registry_by_key_filters() { + let mut registry = FlagRegistry::new(); + registry.record(FlagEntry { + path: "project".to_owned(), + key: "flag-a".to_owned(), + stage: Stage::Beta, + visible: true, + }); + registry.record(FlagEntry { + path: "project:list".to_owned(), + key: "flag-a".to_owned(), + stage: Stage::Beta, + visible: true, + }); + registry.record(FlagEntry { + path: "domain".to_owned(), + key: "flag-b".to_owned(), + stage: Stage::Experimental, + visible: false, + }); + + let matches = registry.by_key("flag-a"); + assert_eq!(matches.len(), 2); + assert!(matches.iter().all(|entry| entry.key == "flag-a")); + + assert!(registry.by_key("no-such-flag").is_empty()); + } +} diff --git a/src/flag_commands.rs b/src/flag_commands.rs new file mode 100644 index 0000000..866a223 --- /dev/null +++ b/src/flag_commands.rs @@ -0,0 +1,112 @@ +//! Built-in `flags` command group: introspection for declared feature flags. +//! +//! Mounted unconditionally by [`crate::Cli::new`] (feature-flag introspection does +//! not depend on any other opt-in system, unlike `env` or `config`). The group +//! exposes: +//! +//! - `flags list` — list every flagged command/group/module node discovered while +//! pruning the command tree, with the stage and visibility decided for this run. +//! - `flags info ` — show the active policy for one flag key plus every node +//! that resolved to it. + +use serde_json::json; + +use crate::{ + CommandResult, CommandSpec, GroupSpec, RuntimeCommandSpec, RuntimeGroupSpec, + error::CliCoreError, +}; + +/// Builds the built-in `flags` command group. +#[must_use] +pub fn flags_command_group() -> RuntimeGroupSpec { + RuntimeGroupSpec::new(GroupSpec::new("flags", "Inspect declared feature flags")) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("list", "List every flagged command/group/module node") + .no_auth(true) + .with_system("flags"), + async |ctx| { + let items: Vec<_> = ctx + .middleware + .flag_registry + .entries() + .iter() + .map(|entry| { + json!({ + "path": entry.path, + "key": entry.key, + "stage": entry.stage.as_str(), + "visible": entry.visible, + }) + }) + .collect(); + Ok(CommandResult::new(json!(items))) + }, + )) + .with_command(RuntimeCommandSpec::new_with_context( + CommandSpec::new("info", "Show the policy and nodes for one flag key") + .no_auth(true) + .with_system("flags") + .with_arg( + clap::Arg::new("key") + .value_name("KEY") + .required(true) + .help("Flag key to inspect"), + ), + async |ctx| { + let key = string_arg(&ctx.args, "key"); + if key.is_empty() { + return Err(CliCoreError::message("missing flag key")); + } + let matches = ctx.middleware.flag_registry.by_key(&key); + if matches.is_empty() { + return Err(CliCoreError::message(format!("no such flag: {key}"))); + } + let has_override = ctx.middleware.flag_policy.overrides.contains_key(&key); + let min_stage = ctx.middleware.flag_policy.min_stage; + let override_stage = ctx + .middleware + .flag_policy + .overrides + .get(&key) + .map(|stage| json!(stage.as_str())) + .unwrap_or(serde_json::Value::Null); + let entries: Vec<_> = matches + .iter() + .map(|entry| { + // An override only "decides" this entry's visibility if removing + // it would flip the outcome; otherwise min_stage alone explains + // it, even though an override for this key happens to exist. + let visible_without_override = entry.stage >= min_stage; + let decided_by = + if has_override && visible_without_override != entry.visible { + "override" + } else { + "min_stage" + }; + json!({ + "path": entry.path, + "stage": entry.stage.as_str(), + "visible": entry.visible, + "decided_by": decided_by, + }) + }) + .collect(); + Ok(CommandResult::new(json!({ + "key": key, + "policy": { + "min_stage": ctx.middleware.flag_policy.min_stage.as_str(), + "override": override_stage, + }, + "entries": entries, + }))) + }, + )) +} + +/// Reads a required string argument, defaulting to empty when absent. +fn string_arg(args: &serde_json::Map, name: &str) -> String { + args.get(name) + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .to_owned() +} diff --git a/src/lib.rs b/src/lib.rs index dd3b999..45ee37b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -77,6 +77,10 @@ mod env_commands; pub mod environments; /// Shared error type and error traits. pub mod error; +/// Stage-based feature flagging primitives (readiness gating), distinct from `flags`. +pub mod feature_flags; +/// Built-in `flags` command group (private; only `cli.rs` consumes it). +mod flag_commands; /// Global framework flags and flag-extraction helpers. pub mod flags; /// Filesystem and path utilities (base dir, path-component safety, atomic write). @@ -128,6 +132,7 @@ pub use environments::{Environment, EnvironmentDef, Environments, OAuthConfig}; pub use error::{ CliCoreError, DetailedError, ExitCoder, Result, exit_code_for_error, exit_code_for_exit_coder, }; +pub use feature_flags::{FeatureFlag, FlagEntry, FlagPolicy, FlagRegistry, Stage}; pub use flags::{ GlobalFlags, app_id_env_prefix, debug_component_enabled, default_output_format, derive_bool_flags, derive_value_flags, extract_command_path, extract_output_format, diff --git a/src/middleware.rs b/src/middleware.rs index ec0c276..37ca255 100644 --- a/src/middleware.rs +++ b/src/middleware.rs @@ -11,7 +11,8 @@ use serde_json::{Map, Value, json}; use tokio::sync::{Mutex, OnceCell}; use crate::{ - CommandResult, Credential, CredentialRequest, Dispatcher, Result, SchemaRegistry, Tier, + CommandResult, Credential, CredentialRequest, Dispatcher, FlagPolicy, FlagRegistry, Result, + SchemaRegistry, Tier, error::{CliCoreError, exit_code_for_error}, output::{ Envelope, HumanViewRegistry, OutputFormat, PipelineOpts, apply_pipeline, @@ -521,6 +522,19 @@ pub struct Middleware { /// active environment through /// [`CommandContext::environment`](crate::command::CommandContext::environment). pub environments: Option>, + /// Merged feature-flag visibility policy for this run. + /// + /// Set by [`CliConfig`](crate::CliConfig)'s `min_stage`/`feature_overrides` + /// (via its private `flag_policy()` helper) when [`Cli::new`](crate::Cli::new) + /// builds middleware, before any module or group is registered. Command-tree + /// pruning consults this to decide which flagged commands, groups, and + /// modules remain mounted. + pub flag_policy: FlagPolicy, + /// Every flagged module/group/command path discovered while pruning the + /// command tree, populated as modules and groups are registered. + /// + /// Powers `flags list`/`flags info` introspection. + pub flag_registry: FlagRegistry, } /// Rendered result produced by middleware. diff --git a/src/module.rs b/src/module.rs index 2a383b0..2ea547b 100644 --- a/src/module.rs +++ b/src/module.rs @@ -3,8 +3,8 @@ use std::{path::Path, sync::Arc}; use schemars::JsonSchema; use crate::{ - GuideEntry, HumanViewDef, Middleware, OutputSchema, RuntimeGroupSpec, SchemaRegistry, - parse_guides_from_markdown, + FeatureFlag, GuideEntry, HumanViewDef, Middleware, OutputSchema, RuntimeGroupSpec, + SchemaRegistry, Stage, parse_guides_from_markdown, }; /// Function used by closure-based modules to register a runtime command group. @@ -45,6 +45,21 @@ pub struct Module { pub guides: Vec, /// Human output views registered before command execution. pub views: Vec, + /// This module's own feature-flag declaration, if any. + /// + /// `None` means the module has no explicit stage declaration of its own, + /// in which case its group and descendants inherit their effective stage + /// from their nearest ancestor (nested group, then enclosing group, then + /// module — nearest declaration wins), implicitly resolving to + /// [`Stage::Ga`] if nothing in the ancestor chain declares a flag either; + /// see [`Stage`]'s documentation for why that is its default. A module is + /// the top-level ancestor in that chain: nothing sits above it. Set with + /// [`with_feature_flag`](Module::with_feature_flag). This field only + /// records the module's own declaration; cascading resolution (and + /// pruning of nodes the active [`FlagPolicy`](crate::FlagPolicy) hides) + /// happens when a [`Cli`](crate::Cli) mounts this module via + /// [`Cli::add_module`](crate::Cli::add_module). + pub feature_flag: Option, /// Registration function that returns the module's runtime group. pub register: ModuleRegister, } @@ -60,6 +75,7 @@ impl Module { category: category.into(), guides: Vec::new(), views: Vec::new(), + feature_flag: None, register: Arc::new(register), } } @@ -78,6 +94,7 @@ impl Module { category, guides, views, + feature_flag: None, register: Arc::new(move |context| module.register(context)), } } @@ -111,6 +128,14 @@ impl Module { self.views.push(view); self } + + /// Declares this module's own feature flag: the key used for policy + /// overrides and introspection, and the stage at which it becomes visible. + #[must_use] + pub fn with_feature_flag(mut self, key: impl Into, stage: Stage) -> Self { + self.feature_flag = Some(FeatureFlag::new(key, stage)); + self + } } impl std::fmt::Debug for Module { @@ -120,6 +145,7 @@ impl std::fmt::Debug for Module { .field("category", &self.category) .field("guides", &self.guides) .field("views", &self.views) + .field("feature_flag", &self.feature_flag) .finish_non_exhaustive() } } @@ -213,3 +239,31 @@ impl<'middleware> ModuleContext<'middleware> { (self.guides, self.views) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::GroupSpec; + + fn trivial_module(category: &str) -> Module { + Module::new(category.to_string(), |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("g", "short")) + }) + } + + #[test] + fn module_with_feature_flag_sets_key_and_stage() { + let module = trivial_module("cat").with_feature_flag("my-module-flag", Stage::Beta); + + let flag = module.feature_flag.expect("feature flag should be set"); + assert_eq!(flag.key, "my-module-flag"); + assert_eq!(flag.stage, Stage::Beta); + } + + #[test] + fn module_feature_flag_defaults_to_none() { + let module = trivial_module("cat"); + + assert!(module.feature_flag.is_none()); + } +} diff --git a/tests/feature_flags.rs b/tests/feature_flags.rs new file mode 100644 index 0000000..6d59ca2 --- /dev/null +++ b/tests/feature_flags.rs @@ -0,0 +1,506 @@ +//! End-to-end coverage for the feature-flagging system, driven entirely through +//! [`Cli::run`] the way a real consumer binary would: cascading resolution across +//! module/group/command with an override, pruning a hidden node from `--help` and +//! `--schema` (and the unknown-command error on direct invocation), environment +//! `min_stage`/`feature_overrides` layering (compiled and via an `_*` env +//! var), and the built-in `flags list`/`flags info` introspection commands. +//! +//! Cascading resolution, pruning internals, and env-var precedence already have +//! thorough unit coverage in `src/cli.rs` and `src/environments.rs`; this file +//! proves the same mechanism end to end through the only surface a real consumer +//! CLI uses, rather than re-testing those internals directly. +#![allow(unsafe_code)] +// The env-var test holds `ENV_LOCK` across an `.await` on purpose, to keep the +// mutation race-free for the whole `Cli::new` + `Cli::run` sequence. +#![allow(clippy::await_holding_lock)] + +use std::sync::{Arc, Mutex, MutexGuard}; + +use cli_engine::{ + Cli, CliConfig, CommandResult, CommandSpec, EnvironmentDef, Environments, GroupSpec, Module, + RuntimeCommandSpec, RuntimeGroupSpec, Stage, +}; +use serde_json::{Value, json}; + +/// A no-op, unauthenticated leaf command used to populate the fixture trees. +fn trivial_command(name: &'static str) -> RuntimeCommandSpec { + RuntimeCommandSpec::new( + CommandSpec::new(name, "Fixture command").no_auth(true), + async |_credential, _args| Ok(CommandResult::new(json!({ "ok": true }))), + ) +} + +/// Builds a `devkit` module: an unflagged top-level group with an unflagged +/// `status` command, and a nested `sandbox` group flagged Experimental whose +/// `peek` command has no flag of its own (so it inherits `sandbox`'s flag). +/// +/// `status` is a sibling of the flagged `sandbox` group (not of `peek`): per the +/// engine's pruning semantics an invisible ancestor drops its whole subtree +/// unconditionally, so `peek` could never survive as a *direct* sibling of an +/// inherited-and-hidden command under the same flagged parent. Nesting the flag +/// one level below `devkit` is what lets `status` demonstrate an unrelated, +/// always-visible command surviving right next to a fully pruned subtree. +fn gated_module() -> Module { + Module::new("Feature Flag Fixtures", |_ctx| { + RuntimeGroupSpec::new(GroupSpec::new("devkit", "Devkit commands")) + .with_group( + RuntimeGroupSpec::new( + GroupSpec::new("sandbox", "Experimental sandbox tools") + .with_feature_flag("sandbox-flag", Stage::Experimental), + ) + .with_command(trivial_command("peek")), + ) + .with_command(trivial_command("status")) + }) +} + +#[tokio::test] +async fn cascading_flag_prunes_inherited_descendant_but_keeps_unflagged_sibling() { + // Default policy: `CliConfig` leaves `min_stage` at its `Stage::Ga` default, + // so the Experimental `sandbox` subgroup (and everything it cascades into) + // must be pruned, while the unrelated `status` command stays. + let cli = Cli::new( + CliConfig::new("flagcascade", "Flag cascade test", "flagcascade-app") + .with_module(gated_module()), + ); + + let help = cli.run(["flagcascade", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("status"), "{}", help.rendered); + assert!(!help.rendered.contains("sandbox"), "{}", help.rendered); + + // The pruned command was never mounted into the clap tree, so dispatching + // it directly fails the same way a typo'd command would. + let dispatch = cli.run(["flagcascade", "devkit", "sandbox", "peek"]).await; + assert_ne!(dispatch.exit_code, 0, "{}", dispatch.rendered); + assert!( + dispatch.rendered.contains("unknown command"), + "{}", + dispatch.rendered + ); + + // `--schema` does not resurrect it either: the path still resolves to + // "unknown command", not a schema (or no-schema) envelope. + let schema = cli + .run([ + "flagcascade", + "devkit", + "sandbox", + "peek", + "--schema", + "--output", + "json", + ]) + .await; + assert_ne!(schema.exit_code, 0, "{}", schema.rendered); + assert!( + schema.rendered.contains("unknown command"), + "{}", + schema.rendered + ); + + // The unflagged sibling dispatches normally. + let visible = cli + .run(["flagcascade", "devkit", "status", "--output", "json"]) + .await; + assert_eq!(visible.exit_code, 0, "{}", visible.rendered); + let visible_json: Value = serde_json::from_str(&visible.rendered).expect("json output"); + assert_eq!(visible_json["data"]["ok"], true); +} + +#[tokio::test] +async fn permissive_min_stage_reveals_previously_pruned_subtree() { + // Same tree, but the consumer opts into Experimental visibility, so the + // subgroup and its inheriting command are mounted and dispatchable. + let cli = Cli::new( + CliConfig::new( + "flagpermissive", + "Flag permissive test", + "flagpermissive-app", + ) + .with_min_stage(Stage::Experimental) + .with_module(gated_module()), + ); + + let help = cli.run(["flagpermissive", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("sandbox"), "{}", help.rendered); + + let dispatch = cli + .run([ + "flagpermissive", + "devkit", + "sandbox", + "peek", + "--output", + "json", + ]) + .await; + assert_eq!(dispatch.exit_code, 0, "{}", dispatch.rendered); +} + +#[tokio::test] +async fn environment_min_stage_loosens_consumer_policy_end_to_end() { + // The `CliConfig` itself stays at its Ga default; only the active + // ("flagtest-envmin") environment's compiled `min_stage` loosens visibility. + // Proves the environment layer reaches pruning through the full `Cli::new` + + // `Cli::run` pipeline, not just `Environments::resolve` in isolation. + // + // The environment name is deliberately test-scoped (not a real name like + // "prod") so its derived `FLAGTEST_ENVMIN_MIN_STAGE` env var cannot collide + // with an `_MIN_STAGE` a developer/CI might have set for a real + // environment, which would otherwise silently override the compiled + // `min_stage` this test asserts on. This test therefore needs no `ENV_LOCK`. + let cli = Cli::new( + CliConfig::new("flagenv", "Flag environment test", "flagenv-app") + .with_environments(Arc::new( + Environments::new("flagtest-envmin").with_environment( + "flagtest-envmin", + EnvironmentDef::new().with_min_stage(Stage::Experimental), + ), + )) + .with_module(gated_module()), + ); + + let help = cli.run(["flagenv", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("sandbox"), "{}", help.rendered); + + let dispatch = cli + .run(["flagenv", "devkit", "sandbox", "peek", "--output", "json"]) + .await; + assert_eq!(dispatch.exit_code, 0, "{}", dispatch.rendered); +} + +#[tokio::test] +async fn environment_feature_override_reveals_pruned_subtree_end_to_end() { + // Distinct from the environment `min_stage` layer above: here both the + // `CliConfig` and the environment leave `min_stage` at Ga, and it is the + // environment's compiled per-key `feature_overrides` entry (forcing + // `sandbox-flag` to Ga) that lifts the otherwise-Experimental subgroup into + // visibility. Proves the environment feature-override layer (not just + // `min_stage`) reaches pruning through the full `Cli::new` + `Cli::run` + // pipeline. + // + // The environment name is test-scoped for the same collision reason as the + // `min_stage` test above: its derived `FLAGTEST_FEATOVERRIDE_*` env vars + // cannot clash with a real environment's, so no `ENV_LOCK` is needed. + let cli = Cli::new( + CliConfig::new( + "flagenvoverride", + "Flag environment override test", + "flagenvoverride-app", + ) + .with_environments(Arc::new( + Environments::new("flagtest-featoverride").with_environment( + "flagtest-featoverride", + EnvironmentDef::new().with_feature_override("sandbox-flag", Stage::Ga), + ), + )) + .with_module(gated_module()), + ); + + let help = cli.run(["flagenvoverride", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("sandbox"), "{}", help.rendered); + + let dispatch = cli + .run([ + "flagenvoverride", + "devkit", + "sandbox", + "peek", + "--output", + "json", + ]) + .await; + assert_eq!(dispatch.exit_code, 0, "{}", dispatch.rendered); +} + +#[tokio::test] +async fn environment_min_stage_tightens_permissive_consumer_policy_end_to_end() { + // The tightening direction, opposite every test above: the `CliConfig` is + // *permissive* (`min_stage` = Experimental, which on its own reveals the + // Experimental `sandbox` subgroup), but the active environment raises + // `min_stage` back up to Ga and re-hides it. Proves the environment layer's + // unconditional replace in `Cli::new` can strengthen — not only loosen — the + // consumer's compiled policy, all the way through pruning and dispatch. + // + // Compiled-only environment, so no env var and no `ENV_LOCK`; the name is + // test-scoped for the same collision reason as the loosening tests above. + let cli = Cli::new( + CliConfig::new("flagtighten", "Flag tighten test", "flagtighten-app") + .with_min_stage(Stage::Experimental) + .with_environments(Arc::new( + Environments::new("flagtest-tighten").with_environment( + "flagtest-tighten", + EnvironmentDef::new().with_min_stage(Stage::Ga), + ), + )) + .with_module(gated_module()), + ); + + // `sandbox` would be visible under the consumer's Experimental floor alone, + // but the environment's Ga floor prunes it again. + let help = cli.run(["flagtighten", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("status"), "{}", help.rendered); + assert!(!help.rendered.contains("sandbox"), "{}", help.rendered); + + // And the re-hidden node is not dispatchable: it was never mounted. + let dispatch = cli + .run([ + "flagtighten", + "devkit", + "sandbox", + "peek", + "--output", + "json", + ]) + .await; + assert_ne!(dispatch.exit_code, 0, "{}", dispatch.rendered); + assert!( + dispatch.rendered.contains("unknown command"), + "{}", + dispatch.rendered + ); +} + +/// Serializes this file's env-var mutations across parallel test threads. +static ENV_LOCK: Mutex<()> = Mutex::new(()); + +fn lock() -> MutexGuard<'static, ()> { + ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// RAII guard that restores (or removes) an env var on drop, even on panic. +struct EnvGuard { + key: &'static str, + prev: Option, +} + +impl EnvGuard { + /// Sets `key` to `value`. Caller must hold [`ENV_LOCK`] for the guard's + /// entire lifetime. + fn set(key: &'static str, value: &str) -> Self { + let prev = std::env::var_os(key); + // SAFETY: serialized by ENV_LOCK; guard restores/removes on any exit + // incl. panic. + unsafe { std::env::set_var(key, value) }; + Self { key, prev } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: serialized by ENV_LOCK; guard restores/removes on any exit + // incl. panic. + unsafe { + match &self.prev { + Some(v) => std::env::set_var(self.key, v), + None => std::env::remove_var(self.key), + } + } + } +} + +// A distinctive, test-scoped environment name so its derived env-var prefix +// (`FLAGTEST_ENVVAR_*`) cannot collide with a real environment name a developer +// might have configured locally. +const ENV_VAR_ENV_NAME: &str = "flagtest-envvar"; +const ENV_VAR_MIN_STAGE: &str = "FLAGTEST_ENVVAR_MIN_STAGE"; + +#[tokio::test] +async fn env_var_min_stage_override_reaches_dispatch_end_to_end() { + // Confirms the full `_MIN_STAGE` env var -> `Environments::resolve` -> + // `FlagPolicy` -> tree-pruning -> clap chain, not just that `resolve()` + // returns the right `Environment` in isolation (already covered in + // `src/environments.rs`'s unit tests). + let _lock = lock(); + let _guard = EnvGuard::set(ENV_VAR_MIN_STAGE, "experimental"); + + let cli = Cli::new( + CliConfig::new("flagenvvar", "Flag env-var test", "flagenvvar-app") + .with_environments(Arc::new( + Environments::new(ENV_VAR_ENV_NAME) + .with_environment(ENV_VAR_ENV_NAME, EnvironmentDef::new()), + )) + .with_module(gated_module()), + ); + + let help = cli.run(["flagenvvar", "devkit"]).await; + assert_eq!(help.exit_code, 0, "{}", help.rendered); + assert!(help.rendered.contains("sandbox"), "{}", help.rendered); + + let dispatch = cli + .run([ + "flagenvvar", + "devkit", + "sandbox", + "peek", + "--output", + "json", + ]) + .await; + assert_eq!(dispatch.exit_code, 0, "{}", dispatch.rendered); +} + +/// Builds a module whose group carries its own feature flag (so it, and its +/// unflagged `list` command, both cascade to `key`/`stage`). +fn flagged_module(group_name: &'static str, key: &'static str, stage: Stage) -> Module { + Module::new("Flags Introspection Fixtures", move |_ctx| { + RuntimeGroupSpec::new( + GroupSpec::new(group_name, "Introspection fixture group").with_feature_flag(key, stage), + ) + .with_command(trivial_command("list")) + }) +} + +#[tokio::test] +async fn flags_list_and_info_report_override_and_min_stage_decisions_end_to_end() { + // One flag key is forced visible by a consumer-level override despite its + // own Experimental declaration; the other relies solely on `min_stage`. + // `flags list`/`flags info` should distinguish the two via `decided_by`. + let cli = Cli::new( + CliConfig::new("flagintro", "Flags introspection test", "flagintro-app") + .with_min_stage(Stage::Beta) + .with_feature_override("override-key", Stage::Ga) + .with_module(flagged_module( + "override-group", + "override-key", + Stage::Experimental, + )) + .with_module(flagged_module( + "min-stage-group", + "min-stage-key", + Stage::Beta, + )), + ); + + let list = cli + .run(["flagintro", "flags", "list", "--output", "json"]) + .await; + assert_eq!(list.exit_code, 0, "{}", list.rendered); + let rendered: Value = serde_json::from_str(&list.rendered).expect("json output"); + let entries = rendered["data"].as_array().expect("data should be array"); + + let override_entry = entries + .iter() + .find(|entry| entry["path"] == "override-group:list") + .expect("override-decided command entry present"); + assert_eq!(override_entry["key"], "override-key"); + assert_eq!(override_entry["stage"], "experimental"); + assert_eq!(override_entry["visible"], true); + + let min_stage_entry = entries + .iter() + .find(|entry| entry["path"] == "min-stage-group:list") + .expect("min-stage-decided command entry present"); + assert_eq!(min_stage_entry["key"], "min-stage-key"); + assert_eq!(min_stage_entry["stage"], "beta"); + assert_eq!(min_stage_entry["visible"], true); + + let override_info = cli + .run([ + "flagintro", + "flags", + "info", + "override-key", + "--output", + "json", + ]) + .await; + assert_eq!(override_info.exit_code, 0, "{}", override_info.rendered); + let override_data: Value = serde_json::from_str(&override_info.rendered).expect("json output"); + assert_eq!(override_data["data"]["policy"]["override"], "ga"); + assert!( + override_data["data"]["entries"] + .as_array() + .expect("entries should be array") + .iter() + .all(|entry| entry["decided_by"] == "override") + ); + + let min_stage_info = cli + .run([ + "flagintro", + "flags", + "info", + "min-stage-key", + "--output", + "json", + ]) + .await; + assert_eq!(min_stage_info.exit_code, 0, "{}", min_stage_info.rendered); + let min_stage_data: Value = + serde_json::from_str(&min_stage_info.rendered).expect("json output"); + assert!(min_stage_data["data"]["policy"]["override"].is_null()); + assert!( + min_stage_data["data"]["entries"] + .as_array() + .expect("entries should be array") + .iter() + .all(|entry| entry["decided_by"] == "min_stage") + ); + + let unknown = cli.run(["flagintro", "flags", "info", "no-such-key"]).await; + assert_ne!(unknown.exit_code, 0, "{}", unknown.rendered); + assert!( + unknown.rendered.contains("no such flag"), + "{}", + unknown.rendered + ); +} + +#[tokio::test] +async fn flags_info_decides_per_entry_when_multiple_nodes_share_a_key() { + // Two unrelated nodes declare the same key with different stages. The + // override (to Beta) flips one node's outcome (Experimental -> visible) + // but not the other's (Beta was already >= min_stage). `decided_by` must + // reflect that per entry, not uniformly for the whole key. + let cli = Cli::new( + CliConfig::new("flagintro2", "Flags introspection test", "flagintro2-app") + .with_min_stage(Stage::Beta) + .with_feature_override("shared-key", Stage::Beta) + .with_module(flagged_module( + "already-visible-group", + "shared-key", + Stage::Beta, + )) + .with_module(flagged_module( + "flipped-group", + "shared-key", + Stage::Experimental, + )), + ); + + let info = cli + .run([ + "flagintro2", + "flags", + "info", + "shared-key", + "--output", + "json", + ]) + .await; + assert_eq!(info.exit_code, 0, "{}", info.rendered); + let data: Value = serde_json::from_str(&info.rendered).expect("json output"); + let entries = data["data"]["entries"].as_array().expect("entries array"); + + let already_visible = entries + .iter() + .find(|entry| entry["path"] == "already-visible-group:list") + .expect("already-visible entry present"); + assert_eq!(already_visible["visible"], true); + assert_eq!(already_visible["decided_by"], "min_stage"); + + let flipped = entries + .iter() + .find(|entry| entry["path"] == "flipped-group:list") + .expect("flipped entry present"); + assert_eq!(flipped["visible"], true); + assert_eq!(flipped["decided_by"], "override"); +}