diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a590e534..7b9ddc62b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `apm install` enforces org `apm-policy.yml` at install time (deps deny/allow/require, MCP deny/transport/trust-transitive, `compilation.target.allow`, `extends:` chains, `policy.fetch_failure` knob, `policy.hash` pin); `--no-policy` / `APM_POLICY_DISABLE=1` escape hatch; `--dry-run` previews verdicts; failed package installs roll back `apm.yml`. New `apm policy status` diagnostic (table / `--json`, exit-0 by default, `--check` for CI). `apm audit --ci` auto-discovers org policy. **Migration**: orgs publishing `enforcement: block` may see installs that previously succeeded now fail -- preview with `apm install --dry-run`. Closes #827, #829, #831, #834 (#832) +- `apm experimental` command group - a feature-flag registry with `list` / `enable` / `disable` / `reset` subcommands. Opt in to new behaviour before it graduates to default. Ships with one built-in flag (`verbose-version`) and a contributor recipe for proposing new flags (#845) - `pr-review-panel` gh-aw workflow: runs the `apm-review-panel` skill on PRs labelled `panel-review` and posts a synthesized verdict (#824) ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e1bc6aa25..4e83414a8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,6 +144,44 @@ uv run isort . If your changes affect how users interact with the project, update the documentation accordingly. +## Extending APM + +### How to add an experimental feature flag + +Use an experimental flag to de-risk rollout of a user-visible behavioural change that may need early adopter feedback. Do not add a flag for a bug fix, internal refactor, or any change that should simply ship as the default behaviour. + +Experimental flags MUST NOT gate security-critical behaviour (content scanning, path validation, lockfile integrity, token handling, MCP trust, collision detection). Flags are ergonomic/UX toggles only. + +When adding a new experimental flag: + +1. Register it in `src/apm_cli/core/experimental.py` in the `FLAGS` dict with a frozen `ExperimentalFlag(name=..., description=..., default=False, hint=...)`. +2. Gate the code path with a function-scope import (avoids import cycles): + ```python + def my_function(): + from apm_cli.core.experimental import is_enabled + if is_enabled("my_flag"): + ... + ``` +3. Add tests that cover both the enabled and disabled code paths. +4. Update the experimental command reference page at `docs/src/content/docs/reference/experimental.md`. + +Naming rules: + +- Use `snake_case` in the registry and config. +- Use `kebab-case` for display and other user-facing strings. +- The CLI accepts both forms on input. + +Graduation and retirement: + +1. When a flag becomes the default, remove the gate and remove the matching `FLAGS` entry in the same PR. +2. Add a `CHANGELOG.md` entry under `Changed` with a migration note if the previous default differed. + +Avoid these anti-patterns: + +- Do not gate security-critical behaviour behind an experimental flag. +- Do not read `is_enabled()` at module import time. +- Do not persist flag state anywhere other than `~/.apm/config.json` via `update_config`. + ## License By contributing to this project, you agree that your contributions will be licensed under the project's [MIT License](LICENSE). @@ -152,4 +190,4 @@ By contributing to this project, you agree that your contributions will be licen If you have any questions, feel free to open an issue or reach out to the maintainers. -Thank you for your contributions! \ No newline at end of file +Thank you for your contributions! diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index 7651b04c1..01b4691f5 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -1713,3 +1713,7 @@ apm runtime status - Runtime preference order (copilot → codex → llm) - Currently active runtime - Next steps if no runtime is available + +## Experimental Features + +`apm experimental` manages opt-in flags that gate new or changing behaviour. Subcommands: `list`, `enable`, `disable`, `reset`. `apm experimental list` also supports `--json`, and `-v` / `--verbose` works on each subcommand. See the full reference in [Experimental Flags](../experimental/). diff --git a/docs/src/content/docs/reference/experimental.md b/docs/src/content/docs/reference/experimental.md new file mode 100644 index 000000000..8b261b8c2 --- /dev/null +++ b/docs/src/content/docs/reference/experimental.md @@ -0,0 +1,188 @@ +--- +title: "apm experimental" +description: "Manage opt-in experimental feature flags. Evaluate new or changing behaviour without affecting APM defaults." +sidebar: + order: 5 + label: "Experimental Flags" +--- + +`apm experimental` manages opt-in feature flags that gate new or changing behaviour. Flags let you evaluate a capability before it graduates to default, and can be toggled at any time without reinstalling APM. + +Default APM behaviour never changes based on what is available here. A flag must be explicitly enabled to take effect, and every flag ships disabled. + +:::caution[Scope] +Experimental flags are ergonomic and UX toggles only. They MUST NOT gate security-critical behaviour -- content scanning, path validation, lockfile integrity, token handling, MCP trust, or collision detection are never placed behind a flag. See [Security Model](../../enterprise/security/). +::: + +## Subcommands + +### `apm experimental list` + +List every registered flag with its current state. This is the default when no subcommand is given. Normal output is just the table; add `--verbose` to also print the config path and the introductory preamble. + +```bash +apm experimental list [OPTIONS] +``` + +**Options:** +- `--enabled` - Show only flags that are currently enabled. +- `--disabled` - Show only flags that are currently disabled. +- `--json` - Emit a JSON array to stdout with `name`, `enabled`, `default`, `description`, and `source` fields. +- `-v, --verbose` - Print the config file path used for overrides and the introductory preamble. + +**Example:** + +```bash +$ apm experimental list + Experimental Features + Flag Status Description + verbose-version disabled Show Python version, platform, and install path in 'apm --version'. + Tip: apm experimental enable +``` + +Verbose output keeps the same table and adds the extra context lines: + +```bash +$ apm experimental list --verbose +Config file: ~/.apm/config.json +Experimental features let you try new behaviour before it becomes default. +...table output... +``` + +Use `--json` for scripts and automation. It suppresses the table, colour, and intro preamble, and still honours `--enabled` / `--disabled` filters: + +```bash +$ apm experimental list --json +[ + { + "name": "verbose_version", + "enabled": false, + "default": false, + "description": "Show Python version, platform, and install path in 'apm --version'.", + "source": "default" + } +] +``` + +The JSON `name` field uses the canonical registry key. For command arguments, APM still accepts either kebab-case (`verbose-version`) or snake_case (`verbose_version`). For clean machine-readable stdout, use `--json` without `--verbose`. + +### `apm experimental enable` + +Enable a flag. The override is persisted immediately. + +```bash +apm experimental enable NAME [OPTIONS] +``` + +**Arguments:** +- `NAME` - Flag name. Accepted in either kebab-case (`verbose-version`) or snake_case (`verbose_version`). + +**Options:** +- `-v, --verbose` - Print the config file path used for overrides. + +**Example:** + +```bash +$ apm experimental enable verbose-version +[+] Enabled experimental feature: verbose-version +Run 'apm --version' to see the new output. +``` + +Unknown names produce an error with suggestions drawn from the registered flag list: + +```bash +$ apm experimental enable verbose-versio +[x] Unknown experimental feature: verbose-versio +Did you mean: verbose-version? +Run 'apm experimental list' to see all available features. +``` + +### `apm experimental disable` + +Disable a flag. If the flag was not enabled, this is a no-op. + +```bash +apm experimental disable NAME [OPTIONS] +``` + +**Options:** +- `-v, --verbose` - Print the config file path used for overrides. + +**Example:** + +```bash +$ apm experimental disable verbose-version +[+] Disabled experimental feature: verbose-version +``` + +### `apm experimental reset` + +Remove overrides and restore default state. With no argument, all overrides are cleared; a confirmation prompt lists exactly what will change. Bulk reset also removes malformed overrides for registered flags, such as a string value where a boolean is expected. + +```bash +apm experimental reset [NAME] [OPTIONS] +``` + +**Arguments:** +- `NAME` - Optional. Reset a single flag rather than all of them. + +**Options:** +- `-y, --yes` - Skip the confirmation prompt (bulk reset only). +- `-v, --verbose` - Print the config file path used for overrides. + +**Example:** + +```bash +$ apm experimental reset +This will reset 1 experimental feature to its default: + verbose-version (currently enabled -> disabled) +Proceed? [y/N]: y +[+] Reset all experimental features to defaults +``` + +Single-flag reset does not prompt: + +```bash +$ apm experimental reset verbose-version +[+] Reset verbose-version to default (disabled) +``` + +## Example workflow + +Try a flag, confirm its effect, then revert: + +```bash +# 1. See what is available +apm experimental list + +# 2. Opt in to verbose version output +apm experimental enable verbose-version + +# 3. Observe the new behaviour +apm --version + +# 4. Revert to default +apm experimental reset verbose-version +``` + +## Available flags + +| Name | Description | +|-------------------|----------------------------------------------------------------------------------| +| `verbose-version` | Show Python version, platform, and install path in `apm --version`. | + +New flags are proposed via [CONTRIBUTING.md](https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md#how-to-add-an-experimental-feature-flag) and graduate to default when stable. See the contributor recipe for the full lifecycle. + +## Storage and scope + +Overrides are written to `~/.apm/config.json` under the `experimental` key and persist across CLI invocations. They are global to the user account and do not vary per project or per shell session. The canonical way to clear overrides is `apm experimental reset`; editing the file by hand is supported but unnecessary. + +Pass `-v` / `--verbose` to any subcommand after the subcommand name (for example `apm experimental list --verbose`) to print the config file path in use. + +When a flag's behaviour is considered stable, it graduates: the gated code becomes the default path and the flag is removed from the registry in a future release. + +## Troubleshooting + +- **"Unknown experimental feature"** - the name is not in the registry. Run `apm experimental list` to see the current set. Suggestions printed below the error use fuzzy matching on registered names. +- **Unknown keys in config** - a flag that was enabled on a previous APM version may have been removed or renamed. `apm experimental list` surfaces a note when stale keys are present; `apm experimental reset` clears them. +- **Malformed values in config** - if a registered flag has a non-boolean override in `~/.apm/config.json`, `apm experimental reset --yes` removes the bad value and restores the default. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 70f1d6860..a70f485eb 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -82,6 +82,18 @@ Set `MCP_REGISTRY_URL` (default `https://api.mcp.github.com`) to point all `apm | `apm runtime remove {copilot\|codex\|llm}` | Remove a runtime | `--yes` | | `apm runtime status` | Show active runtime | -- | +## Experimental features + +| Command | Purpose | Key flags | +|---------|---------|-----------| +| `apm experimental` | Default to `apm experimental list` | `-v` verbose | +| `apm experimental list` | List registered experimental flags or emit JSON for automation | `--enabled`, `--disabled`, `--json`, `-v` verbose | +| `apm experimental enable NAME` | Enable an opt-in experimental flag | `-v` verbose | +| `apm experimental disable NAME` | Disable an opt-in experimental flag | `-v` verbose | +| `apm experimental reset [NAME]` | Reset one flag or all flags to defaults; also cleans malformed overrides during bulk reset | `-y` skip confirm, `-v` verbose | + +Experimental flags MUST NOT gate security-critical behaviour (content scanning, path validation, lockfile integrity, token handling, MCP trust, collision detection). Flags are ergonomic/UX toggles only. + ## Configuration and updates | Command | Purpose | Key flags | diff --git a/src/apm_cli/cli.py b/src/apm_cli/cli.py index 929827f78..9380c4912 100644 --- a/src/apm_cli/cli.py +++ b/src/apm_cli/cli.py @@ -20,6 +20,7 @@ from apm_cli.commands.compile import compile as compile_cmd from apm_cli.commands.config import config from apm_cli.commands.deps import deps +from apm_cli.commands.experimental import experimental from apm_cli.commands.view import view as view_cmd from apm_cli.commands.init import init from apm_cli.commands.install import install @@ -81,6 +82,7 @@ def cli(ctx): cli.add_command(preview) cli.add_command(list_cmd, name="list") cli.add_command(config) +cli.add_command(experimental) cli.add_command(runtime) cli.add_command(mcp) cli.add_command(policy) diff --git a/src/apm_cli/commands/_helpers.py b/src/apm_cli/commands/_helpers.py index 48a8bc648..fcb857ebd 100644 --- a/src/apm_cli/commands/_helpers.py +++ b/src/apm_cli/commands/_helpers.py @@ -228,6 +228,25 @@ def print_version(ctx, param, value): f"{TITLE}Agent Package Manager (APM) CLI{RESET} version {version_str}" ) + # Gated verbose-version output (experimental flag) + try: + from ..core.experimental import is_enabled + + if is_enabled("verbose_version"): + import platform + import sys + + python_ver = platform.python_version() + plat = f"{sys.platform}-{platform.machine()}" + install_path = str(Path(__file__).resolve().parent.parent) + + _rich_echo(f" {'Python:':<14}{python_ver}", color="dim") + _rich_echo(f" {'Platform:':<14}{plat}", color="dim") + _rich_echo(f" {'Install path:':<14}{install_path}", color="dim") + except Exception: + # Never let experimental flag logic break --version + pass + ctx.exit() diff --git a/src/apm_cli/commands/experimental.py b/src/apm_cli/commands/experimental.py new file mode 100644 index 000000000..12905e44b --- /dev/null +++ b/src/apm_cli/commands/experimental.py @@ -0,0 +1,349 @@ +"""APM experimental feature-flag command group. + +Provides ``apm experimental list|enable|disable|reset`` to manage +opt-in feature flags stored in ``~/.apm/config.json``. +""" + +import json +import sys + +import click + +from ._helpers import _get_console +from ..core.command_logger import CommandLogger +from ..core.experimental import ( + FLAGS, + display_name, + normalise_flag_name, + validate_flag_name, + enable as _enable_flag, + disable as _disable_flag, + get_malformed_flag_keys, + get_overridden_flags, + get_stale_config_keys, + reset as _reset_flags, +) + + +# ------------------------------------------------------------------ +# Helpers +# ------------------------------------------------------------------ + + +def _resolve_verbose(ctx: click.Context, local_verbose: bool) -> bool: + """Merge subcommand-local ``--verbose`` with the group-level value. + + Prefers the subcommand-local flag when explicitly passed; otherwise + inherits from the group's ``ctx.obj["verbose"]``. + """ + if local_verbose: + return True + return (ctx.obj.get("verbose", False) if ctx.obj else False) + + +def _print_list_footer(flags_shown: list, logger: "CommandLogger") -> None: + """Print the footer hint and stale-key note after the flag listing. + + Shared by both the Rich table and plain-text rendering paths. + """ + from ..core.experimental import is_enabled + + enabled_count = sum(1 for f in flags_shown if is_enabled(f.name)) + if enabled_count == 0: + logger.progress("Tip: apm experimental enable ") + else: + logger.progress("Tip: apm experimental disable to revert") + + stale = get_stale_config_keys() + if stale: + logger.progress( + f"Note: {len(stale)} unknown flag(s) in config " + "(run 'apm experimental reset' to clean up)" + ) + + +def _build_table(flags_to_show, logger): + """Build and print a Rich table of experimental flags. + + Falls back to plain-text output when Rich is unavailable. + """ + from ..core.experimental import is_enabled + + try: + from rich.table import Table + + console = _get_console() + if console is None: + raise ImportError("Rich console unavailable") + + table = Table( + title="Experimental Features", + show_header=True, + header_style="bold cyan", + ) + table.add_column("Flag", style="bold white", min_width=18) + table.add_column("Status", min_width=10) + table.add_column("Description", style="white", min_width=30) + + for flag in flags_to_show: + enabled = is_enabled(flag.name) + status_word = "enabled" if enabled else "disabled" + status_cell = f"[{'green bold' if enabled else 'dim'}]{status_word}[/]" + table.add_row( + display_name(flag.name), + status_cell, + flag.description, + ) + + console.print(table) + _print_list_footer(flags_to_show, logger) + + except ImportError: + # Rich not installed -- plain-text fallback + from ..utils.console import _rich_echo + + for flag in flags_to_show: + enabled = is_enabled(flag.name) + status = "enabled" if enabled else "disabled" + _rich_echo(f" {display_name(flag.name)} [{status}]", color="white", bold=True) + _rich_echo(f" {flag.description}", color="dim") + + _print_list_footer(flags_to_show, logger) + + +def _handle_unknown_flag(name: str, logger: CommandLogger) -> None: + """Handle an unknown flag name: print error, suggestions, and exit. + + Callers are responsible for passing a normalised (snake_case) name. + """ + try: + validate_flag_name(name) + except ValueError as exc: + args = exc.args + display = display_name(name) + logger.error(f"Unknown experimental feature: {display}") + + suggestions = args[1] if len(args) > 1 else [] + if len(suggestions) == 1: + logger.progress(f"Did you mean: {suggestions[0]}?") + elif len(suggestions) > 1: + logger.progress(f"Similar features: {', '.join(suggestions)}") + + logger.progress("Run 'apm experimental list' to see all available features.") + sys.exit(1) + + +# ------------------------------------------------------------------ +# Click command group +# ------------------------------------------------------------------ + +@click.group( + help="Manage experimental feature flags", + invoke_without_command=True, + context_settings={"allow_interspersed_args": True, "ignore_unknown_options": True}, +) +@click.option("--verbose", "-v", is_flag=True, default=False, help="Show verbose output") +@click.pass_context +def experimental(ctx, verbose: bool): + """Manage experimental feature flags.""" + ctx.ensure_object(dict) + ctx.obj["verbose"] = verbose + if ctx.invoked_subcommand is None: + # Default subcommand: list + ctx.invoke(list_flags) + + +@experimental.command("list", help="List all experimental features") +@click.option("--enabled", "filter_enabled", is_flag=True, default=False, help="Show only enabled features") +@click.option("--disabled", "filter_disabled", is_flag=True, default=False, help="Show only disabled features") +@click.option("--verbose", "-v", is_flag=True, default=False, help="Show detailed output") +@click.option("--json", "as_json", is_flag=True, default=False, help="Output as JSON array") +@click.pass_context +def list_flags(ctx, filter_enabled: bool, filter_disabled: bool, verbose: bool, as_json: bool): + """List all registered experimental flags.""" + if filter_enabled and filter_disabled: + raise click.UsageError("--enabled and --disabled are mutually exclusive.") + + verbose = _resolve_verbose(ctx, verbose) + logger = CommandLogger("experimental list", verbose=verbose) + + from ..config import CONFIG_FILE + + logger.verbose_detail(f"Config file: {CONFIG_FILE}") + + all_flags = list(FLAGS.values()) + + if filter_enabled: + from ..core.experimental import is_enabled + + flags_to_show = [f for f in all_flags if is_enabled(f.name)] + if not flags_to_show and not as_json: + logger.progress("No experimental flags are enabled.") + return + elif filter_disabled: + from ..core.experimental import is_enabled + + flags_to_show = [f for f in all_flags if not is_enabled(f.name)] + if not flags_to_show and not as_json: + logger.progress("All experimental flags are currently enabled.") + return + else: + flags_to_show = all_flags + + if as_json: + from ..core.experimental import is_enabled + + overridden = get_overridden_flags() + rows = [] + for flag in flags_to_show: + rows.append({ + "name": flag.name, + "enabled": is_enabled(flag.name), + "default": flag.default, + "description": flag.description, + "source": "config" if flag.name in overridden else "default", + }) + click.echo(json.dumps(rows, indent=2)) + return + + logger.verbose_detail("Experimental features let you try new behaviour before it becomes default.") + _build_table(flags_to_show, logger) + + +@experimental.command("enable", help="Enable an experimental feature") +@click.argument("name") +@click.option("--verbose", "-v", is_flag=True, default=False, help="Show detailed output") +@click.pass_context +def enable_flag(ctx, name: str, verbose: bool): + """Enable an experimental feature flag.""" + verbose = _resolve_verbose(ctx, verbose) + logger = CommandLogger("experimental enable", verbose=verbose) + + from ..config import CONFIG_FILE + + logger.verbose_detail(f"Config file: {CONFIG_FILE}") + + normalised = normalise_flag_name(name) + if normalised not in FLAGS: + _handle_unknown_flag(normalised, logger) + return # unreachable after sys.exit, but satisfies linters + + from ..core.experimental import is_enabled + + if is_enabled(normalised): + logger.warning(f"{display_name(normalised)} is already enabled.") + return + + flag = _enable_flag(normalised) + logger.success(f"Enabled experimental feature: {display_name(normalised)}", symbol="check") + if flag.hint: + logger.progress(flag.hint) + + +@experimental.command("disable", help="Disable an experimental feature") +@click.argument("name") +@click.option("--verbose", "-v", is_flag=True, default=False, help="Show detailed output") +@click.pass_context +def disable_flag(ctx, name: str, verbose: bool): + """Disable an experimental feature flag.""" + verbose = _resolve_verbose(ctx, verbose) + logger = CommandLogger("experimental disable", verbose=verbose) + + from ..config import CONFIG_FILE + + logger.verbose_detail(f"Config file: {CONFIG_FILE}") + + normalised = normalise_flag_name(name) + if normalised not in FLAGS: + _handle_unknown_flag(normalised, logger) + return + + from ..core.experimental import is_enabled + + if not is_enabled(normalised): + logger.warning(f"{display_name(normalised)} is already disabled.") + return + + _disable_flag(normalised) + logger.success(f"Disabled experimental feature: {display_name(normalised)}", symbol="check") + + +@experimental.command("reset", help="Reset experimental features to defaults") +@click.argument("name", required=False, default=None) +@click.option("--yes", "-y", is_flag=True, default=False, help="Skip confirmation prompt") +@click.option("--verbose", "-v", is_flag=True, default=False, help="Show detailed output") +@click.pass_context +def reset_flags(ctx, name: str | None, yes: bool, verbose: bool): + """Reset one or all experimental features to their defaults.""" + verbose = _resolve_verbose(ctx, verbose) + logger = CommandLogger("experimental reset", verbose=verbose) + + from ..config import CONFIG_FILE + + logger.verbose_detail(f"Config file: {CONFIG_FILE}") + + if name is not None: + # Single-flag reset + normalised = normalise_flag_name(name) + if normalised not in FLAGS: + _handle_unknown_flag(normalised, logger) + return + + reset_result = _reset_flags(normalised) + default_label = "enabled" if FLAGS[normalised].default else "disabled" + if reset_result > 0: + logger.success( + f"Reset {display_name(normalised)} to default ({default_label})", + symbol="check", + ) + else: + logger.progress( + f"{display_name(normalised)} is already at its default ({default_label}). Nothing to do." + ) + return + + # Bulk reset -- collect overrides (known bool, stale, and malformed) + overridden = get_overridden_flags() + stale = get_stale_config_keys() + malformed = get_malformed_flag_keys() + + if not overridden and not stale and not malformed: + logger.progress("All features already at default settings. Nothing to reset.") + return + + # Build confirmation listing + if not yes: + lines = [] + for flag_name, value in overridden.items(): + current = "enabled" if value else "disabled" + default = "enabled" if FLAGS[flag_name].default else "disabled" + if current == default: + lines.append(f" {display_name(flag_name)} (redundant override - removing)") + else: + lines.append(f" {display_name(flag_name)} (currently {current} -> {default})") + for key in stale: + lines.append(f" {display_name(key)} (unknown, will be removed)") + for key in malformed: + lines.append(f" {display_name(key)} (malformed value, will be removed)") + + total = len(overridden) + len(stale) + len(malformed) + noun = "feature" if total == 1 else "features" + pronoun = "its" if total == 1 else "their" + default_word = "default" if total == 1 else "defaults" + logger.progress(f"This will reset {total} experimental {noun} to {pronoun} {default_word}:") + for line in lines: + logger.progress(line) + + try: + from rich.prompt import Confirm + + confirmed = Confirm.ask("Proceed?", default=False) + except ImportError: + confirmed = click.confirm("Proceed?", default=False) + + if not confirmed: + logger.progress("Operation cancelled") + return + + _reset_flags(None) + logger.success("Reset all experimental features to defaults", symbol="check") diff --git a/src/apm_cli/core/experimental.py b/src/apm_cli/core/experimental.py new file mode 100644 index 000000000..15939df4f --- /dev/null +++ b/src/apm_cli/core/experimental.py @@ -0,0 +1,266 @@ +"""Experimental feature-flag subsystem for APM CLI. + +Provides a lightweight, static-registry mechanism to gate new or changed +behaviour behind named feature flags. Early adopters can opt-in via +``apm experimental enable `` without branching or separate builds. + +**Caller convention (mandatory):** + + Import ``is_enabled`` at *function scope*, never at module level, to + avoid triggering config I/O at import time for unrelated commands:: + + def my_function(): + from apm_cli.core.experimental import is_enabled + if is_enabled("verbose_version"): + ... + +**Security invariant:** + + Experimental flags MUST NOT gate security-critical behaviour -- content + scanning, path validation, lockfile integrity, token handling, MCP trust + boundary checks, collision detection, or any check documented in + ``enterprise/security.md``. ``~/.apm/config.json`` is user-writable + and carries user-equivalent trust only. +""" + +from __future__ import annotations + +import difflib +from dataclasses import dataclass + + +# --------------------------------------------------------------------------- +# Registry dataclass +# --------------------------------------------------------------------------- + +@dataclass(frozen=True) +class ExperimentalFlag: + """Descriptor for a single experimental feature flag. + + Attributes: + name: Internal snake_case identifier (must match the ``FLAGS`` key). + description: One-line summary (<=80 chars, printable ASCII only). + default: Registry default -- must be ``False`` for every flag. + hint: Optional next-step message shown after a successful ``enable``. + """ + + name: str + description: str + default: bool + hint: str | None = None + + +# --------------------------------------------------------------------------- +# Static registry -- add new flags here +# --------------------------------------------------------------------------- + +FLAGS: dict[str, ExperimentalFlag] = { + "verbose_version": ExperimentalFlag( + name="verbose_version", + description="Show Python version, platform, and install path in 'apm --version'.", + default=False, + hint="Run 'apm --version' to see the new output.", + ), +} + + +# --------------------------------------------------------------------------- +# Name normalisation +# --------------------------------------------------------------------------- + +def normalise_flag_name(name: str) -> str: + """Normalise a CLI flag name to its internal snake_case form. + + Accepts both ``verbose-version`` and ``verbose_version``. + """ + return name.replace("-", "_").lower() + + +def display_name(name: str) -> str: + """Convert an internal snake_case flag name to kebab-case for display.""" + return name.replace("_", "-") + + +# --------------------------------------------------------------------------- +# Config access helper +# --------------------------------------------------------------------------- + +def _get_experimental_section() -> dict: + """Return the ``experimental`` section from config as a dict. + + If the value is not a dict (e.g. user hand-edited the file to an int + or string), returns an empty dict so every consumer fails closed. + """ + from apm_cli.config import get_config + + experimental = get_config().get("experimental", {}) + return experimental if isinstance(experimental, dict) else {} + + +# --------------------------------------------------------------------------- +# Core query +# --------------------------------------------------------------------------- + +def is_enabled(name: str) -> bool: + """Check whether an experimental flag is currently enabled. + + Derives directly from ``get_config()`` (already cached in + ``apm_cli.config._config_cache``). Net cost per call: two dict + lookups after the first config load -- no I/O, no intermediate + object allocation. + + Args: + name: Internal snake_case flag identifier. + + Returns: + ``True`` if the flag is enabled, ``False`` otherwise. + + Raises: + ValueError: If *name* is not a registered flag (fail loud on typos + in shipped code). + """ + if name not in FLAGS: + raise ValueError( + f"Unknown experimental flag: {name!r}. " + f"Registered flags: {', '.join(sorted(FLAGS))}" + ) + + experimental = _get_experimental_section() + + value = experimental.get(name) + # Reject non-bool overrides -- fail closed to registry default. + if not isinstance(value, bool): + return FLAGS[name].default + return value + + +# --------------------------------------------------------------------------- +# Mutators (thin wrappers around apm_cli.config.update_config) +# --------------------------------------------------------------------------- + +def validate_flag_name(name: str) -> str: + """Validate and normalise a flag name from CLI input. + + Returns the normalised snake_case name on success. + + Raises: + ValueError: If the flag is not registered. The exception message + includes ``difflib``-based suggestions when available. + """ + normalised = normalise_flag_name(name) + if normalised in FLAGS: + return normalised + + display = display_name(normalised) + suggestions = difflib.get_close_matches( + normalised, FLAGS.keys(), n=3, cutoff=0.6, + ) + msg = f"Unknown experimental feature: {display}" + raise ValueError(msg, [display_name(s) for s in suggestions]) + + +def _set_flag(name: str, value: bool) -> ExperimentalFlag: + """Set an experimental flag to a bool value and persist the override. + + Args: + name: Snake_case flag identifier (already validated). + value: ``True`` to enable, ``False`` to disable. + + Returns: + The ``ExperimentalFlag`` descriptor for post-mutation messaging. + """ + from apm_cli.config import update_config + + flag = FLAGS[name] + experimental = dict(_get_experimental_section()) + experimental[name] = value + update_config({"experimental": experimental}) + return flag + + +def enable(name: str) -> ExperimentalFlag: + """Enable an experimental flag and persist the override. + + Args: + name: Snake_case flag identifier (already validated). + + Returns: + The ``ExperimentalFlag`` descriptor for post-enable messaging. + """ + return _set_flag(name, True) + + +def disable(name: str) -> ExperimentalFlag: + """Disable an experimental flag and persist the override. + + Args: + name: Snake_case flag identifier (already validated). + + Returns: + The ``ExperimentalFlag`` descriptor for post-disable messaging. + """ + return _set_flag(name, False) + + +def reset(name: str | None = None) -> int: + """Reset one or all experimental flags to their registry defaults. + + When *name* is ``None``, clears all keys from ``experimental`` + (sets it to ``{}``). When *name* is given, removes only that + single key. + + Args: + name: Snake_case flag identifier, or ``None`` for bulk reset. + + Returns: + Number of keys that were actually removed. + """ + from apm_cli.config import update_config + + experimental = dict(_get_experimental_section()) + + if name is not None: + if name in experimental: + del experimental[name] + update_config({"experimental": experimental}) + return 1 + return 0 + + # Bulk reset -- remove all keys + count = len(experimental) + if count: + update_config({"experimental": {}}) + return count + + +def get_overridden_flags() -> dict[str, bool]: + """Return the dict of flags that have user overrides in config. + + Only includes flags that are still registered in ``FLAGS``. + Values are the current override booleans. + """ + experimental = _get_experimental_section() + return { + k: v for k, v in experimental.items() + if k in FLAGS and isinstance(v, bool) + } + + +def get_stale_config_keys() -> list[str]: + """Return config keys under ``experimental`` that are not in ``FLAGS``. + + These are leftovers from removed flags and are safe to clean up via + ``apm experimental reset``. + """ + experimental = _get_experimental_section() + return [k for k in experimental if k not in FLAGS] + + +def get_malformed_flag_keys() -> list[str]: + """Return registered flag names whose config values are not booleans. + + These are known flags with corrupt values (e.g. ``"true"`` instead of + ``True``). They are safe to remove via ``apm experimental reset``. + """ + experimental = _get_experimental_section() + return [k for k in experimental if k in FLAGS and not isinstance(experimental[k], bool)] diff --git a/tests/unit/commands/test_experimental_command.py b/tests/unit/commands/test_experimental_command.py new file mode 100644 index 000000000..0c6935090 --- /dev/null +++ b/tests/unit/commands/test_experimental_command.py @@ -0,0 +1,602 @@ +"""Unit tests for the `apm experimental` CLI command group. + +Uses click.testing.CliRunner exclusively; no real ~/.apm/ writes occur. + +Coverage: + - `apm experimental` (no subcommand) invokes list and shows the table. + - list / list --enabled / list --disabled filtering. + - enable: success message, hint line, underscore input normalisation. + - enable with typo: exit 1, suggestion, recovery hint. + - enable with completely unknown flag: exit 1, no "Did you mean" line. + - disable: success message. + - reset : single-flag reset confirmation. + - reset (no args): nothing-to-reset path, decline confirmation, --yes path. + - -v / --verbose: config file path appears in output. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +from click.testing import CliRunner + + +# --------------------------------------------------------------------------- +# Module-level fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def runner() -> CliRunner: + """CliRunner -- stderr is merged into stdout by default in Click 8.""" + return CliRunner() + + +@pytest.fixture(autouse=True) +def _reset_config_cache() -> None: + """Reset the in-process config cache before and after every test.""" + from apm_cli.config import _invalidate_config_cache + + _invalidate_config_cache() + yield + _invalidate_config_cache() + + +@pytest.fixture(autouse=True) +def _isolate_config(tmp_path, monkeypatch) -> None: + """Redirect all config reads/writes to a throw-away temp directory. + + ``ensure_config_exists()`` will create the directory and file on first + access -- no pre-creation required. + """ + import apm_cli.config as _conf + + config_dir = tmp_path / ".apm" + monkeypatch.setattr(_conf, "CONFIG_DIR", str(config_dir)) + monkeypatch.setattr(_conf, "CONFIG_FILE", str(config_dir / "config.json")) + monkeypatch.setattr(_conf, "_config_cache", None) + + +# --------------------------------------------------------------------------- +# list (default subcommand) +# --------------------------------------------------------------------------- + + +class TestListCommand: + """Tests for `apm experimental list` and the default-to-list behaviour.""" + + def test_no_subcommand_invokes_list_and_shows_table_header( + self, runner: CliRunner + ) -> None: + """Invoking the group with no subcommand defaults to `list`.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, []) + assert result.exit_code == 0 + # The Rich table title or flag name must be present. + assert ( + "Experimental Features" in result.output + or "verbose-version" in result.output + ) + + def test_list_shows_verbose_version_disabled_by_default( + self, runner: CliRunner + ) -> None: + """verbose-version appears with 'disabled' status when no override is set.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list"]) + assert result.exit_code == 0 + assert "verbose-version" in result.output + assert "disabled" in result.output + + def test_list_enabled_filter_prints_no_flags_message_when_none_enabled( + self, runner: CliRunner + ) -> None: + """--enabled with nothing enabled prints the 'no flags enabled' message.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--enabled"]) + assert result.exit_code == 0 + assert "No experimental flags are enabled." in result.output + + def test_list_disabled_filter_shows_flag_at_default( + self, runner: CliRunner + ) -> None: + """--disabled shows verbose-version when it is at its default (disabled).""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--disabled"]) + assert result.exit_code == 0 + assert "verbose-version" in result.output + + def test_list_after_enable_appears_in_enabled_not_in_disabled( + self, runner: CliRunner + ) -> None: + """After enabling, --enabled shows the flag and --disabled does not.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + + # --enabled must show the flag + result_en = runner.invoke(experimental, ["list", "--enabled"]) + assert result_en.exit_code == 0 + assert "verbose-version" in result_en.output + + # --disabled must NOT show the flag (all flags enabled message) + result_dis = runner.invoke(experimental, ["list", "--disabled"]) + assert result_dis.exit_code == 0 + assert "verbose-version" not in result_dis.output + def test_list_enabled_and_disabled_are_mutually_exclusive( + self, runner: CliRunner + ) -> None: + """Passing both --enabled and --disabled produces a UsageError.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--enabled", "--disabled"]) + assert result.exit_code != 0 + assert "mutually exclusive" in result.output + + +# --------------------------------------------------------------------------- +# enable subcommand +# --------------------------------------------------------------------------- + + +class TestEnableCommand: + """Tests for `apm experimental enable `.""" + + def test_enable_exits_0_emits_success_and_hint( + self, runner: CliRunner + ) -> None: + """Successful enable exits 0, emits [+] success line and hint.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["enable", "verbose-version"]) + assert result.exit_code == 0 + assert "[+] Enabled experimental feature: verbose-version" in result.output + # hint line must follow success + assert "apm --version" in result.output + + def test_enable_already_enabled_emits_warning_not_success( + self, runner: CliRunner + ) -> None: + """Enabling an already-enabled flag emits warning [!], not a false success.""" + from apm_cli.commands.experimental import experimental + + # First enable succeeds + runner.invoke(experimental, ["enable", "verbose-version"]) + # Second enable should be idempotent warning + result = runner.invoke(experimental, ["enable", "verbose-version"]) + assert result.exit_code == 0 + assert "[!]" in result.output + assert "already enabled" in result.output + assert "[+] Enabled" not in result.output + + def test_enable_accepts_underscore_input(self, runner: CliRunner) -> None: + """verbose_version (underscore) is normalised and accepted.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["enable", "verbose_version"]) + assert result.exit_code == 0 + assert "Enabled experimental feature: verbose-version" in result.output + + def test_enable_typo_exits_1_with_suggestion_and_recovery_hint( + self, runner: CliRunner + ) -> None: + """One-character typo produces exit 1, error message, suggestion, recovery hint.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["enable", "verbse-version"]) + assert result.exit_code == 1 + assert "Unknown experimental feature: verbse-version" in result.output + assert "Did you mean: verbose-version?" in result.output + assert "apm experimental list" in result.output + + def test_enable_bogus_flag_exits_1_without_suggestion( + self, runner: CliRunner + ) -> None: + """A flag name with no similarity produces no 'Did you mean' line.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["enable", "zzzz-totally-unrelated-qwerty"]) + assert result.exit_code == 1 + assert "Unknown experimental feature" in result.output + assert "Did you mean" not in result.output + + +# --------------------------------------------------------------------------- +# disable subcommand +# --------------------------------------------------------------------------- + + +class TestDisableCommand: + """Tests for `apm experimental disable `.""" + + def test_disable_after_enable_exits_0_emits_success( + self, runner: CliRunner + ) -> None: + """disable exits 0 and emits the [+] disabled confirmation.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + result = runner.invoke(experimental, ["disable", "verbose-version"]) + assert result.exit_code == 0 + assert "[+] Disabled experimental feature: verbose-version" in result.output + + def test_disable_already_disabled_emits_warning_not_success( + self, runner: CliRunner + ) -> None: + """Disabling an already-disabled flag emits warning [!], not a false success.""" + from apm_cli.commands.experimental import experimental + + # Flag is disabled by default -- second disable should be idempotent warning + result = runner.invoke(experimental, ["disable", "verbose-version"]) + assert result.exit_code == 0 + assert "[!]" in result.output + assert "already disabled" in result.output + assert "[+] Disabled" not in result.output + + +# --------------------------------------------------------------------------- +# reset subcommand +# --------------------------------------------------------------------------- + + +class TestResetCommand: + """Tests for `apm experimental reset [name] [--yes]`.""" + + def test_reset_single_flag_exits_0_emits_confirmation( + self, runner: CliRunner + ) -> None: + """reset exits 0 and emits the per-flag reset confirmation.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + result = runner.invoke(experimental, ["reset", "verbose-version"]) + assert result.exit_code == 0 + assert "[+] Reset verbose-version to default" in result.output + + def test_reset_single_flag_already_at_default_prints_noop( + self, runner: CliRunner + ) -> None: + """reset on a pristine config prints nothing-to-do, not success.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["reset", "verbose-version"]) + assert result.exit_code == 0 + assert "already at its default" in result.output + assert "Nothing to do" in result.output + # Must NOT falsely claim a reset occurred + assert "Reset verbose-version to default" not in result.output + + def test_reset_no_overrides_prints_nothing_to_reset( + self, runner: CliRunner + ) -> None: + """reset with no overrides active emits the 'nothing to reset' message.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["reset"]) + assert result.exit_code == 0 + assert ( + "All features already at default settings. Nothing to reset." + in result.output + ) + + def test_reset_with_overrides_declining_confirmation_does_not_reset( + self, runner: CliRunner + ) -> None: + """Declining the confirmation prompt does not call _reset_flags(None).""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + + with patch( + "apm_cli.commands.experimental._reset_flags" + ) as mock_reset, patch("rich.prompt.Confirm.ask", return_value=False): + result = runner.invoke(experimental, ["reset"]) + + assert result.exit_code == 0 + assert "Operation cancelled" in result.output + # Verify the bulk-reset call never happened. + bulk_calls = [c for c in mock_reset.call_args_list if c.args == (None,)] + assert len(bulk_calls) == 0 + + def test_reset_yes_flag_skips_prompt_and_resets( + self, runner: CliRunner + ) -> None: + """--yes bypasses the confirmation prompt and resets all overrides.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + result = runner.invoke(experimental, ["reset", "--yes"]) + assert result.exit_code == 0 + assert "[+] Reset all experimental features to defaults" in result.output + + def test_reset_redundant_override_shows_removing_wording( + self, runner: CliRunner + ) -> None: + """When override equals default, confirmation uses 'redundant override - removing'.""" + from apm_cli.commands.experimental import experimental + + # disable sets the flag to False, which matches default=False -> redundant + runner.invoke(experimental, ["enable", "verbose-version"]) + runner.invoke(experimental, ["disable", "verbose-version"]) + + with patch("rich.prompt.Confirm.ask", return_value=False): + result = runner.invoke(experimental, ["reset"]) + + assert result.exit_code == 0 + assert "redundant override - removing" in result.output + # Must NOT contain the old arrow format for redundant overrides + assert "currently disabled -> disabled" not in result.output + + def test_reset_singular_uses_its_default( + self, runner: CliRunner + ) -> None: + """When resetting exactly 1 flag, summary says 'its default' not 'their defaults'.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + + with patch("rich.prompt.Confirm.ask", return_value=False): + result = runner.invoke(experimental, ["reset"]) + + assert result.exit_code == 0 + assert "its default" in result.output + assert "their defaults" not in result.output + + +# --------------------------------------------------------------------------- +# --verbose flag +# --------------------------------------------------------------------------- + + +class TestVerboseFlag: + """Tests for the -v / --verbose output path.""" + + def test_verbose_list_shows_config_file_path(self, runner: CliRunner) -> None: + """With -v, verbose_detail emits 'Config file: ' before the table.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["-v", "list"]) + assert result.exit_code == 0 + assert "Config file:" in result.output + + def test_verbose_after_subcommand_succeeds(self, runner: CliRunner) -> None: + """apm experimental list -v must not raise 'Error: No such option: -v'.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "-v"]) + assert result.exit_code == 0 + assert "Config file:" in result.output + assert "No such option" not in result.output + + +# --------------------------------------------------------------------------- +# Intro line (SF-2) +# --------------------------------------------------------------------------- + + +class TestIntroLine: + """Tests for the intro-line displayed by `list`.""" + + def test_list_does_not_emit_intro_at_normal_verbosity(self, runner: CliRunner) -> None: + """Normal `list` output does NOT contain the intro preamble.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list"]) + assert result.exit_code == 0 + assert ( + "Experimental features let you try new behaviour" + not in result.output + ) + + def test_list_verbose_emits_intro_line(self, runner: CliRunner) -> None: + """With --verbose, `list` prints the intro description.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--verbose"]) + assert result.exit_code == 0 + assert ( + "Experimental features let you try new behaviour before it becomes default." + in result.output + ) + + +# --------------------------------------------------------------------------- +# --verbose after subcommand (P-UX-1) +# --------------------------------------------------------------------------- + + +class TestVerboseAfterSubcommand: + """Tests that --verbose/-v works when placed AFTER the subcommand name.""" + + def test_enable_verbose_after_subcommand(self, runner: CliRunner) -> None: + """apm experimental enable --verbose shows Config file.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["enable", "verbose-version", "--verbose"]) + assert result.exit_code == 0 + assert "Config file:" in result.output + + def test_disable_verbose_after_subcommand(self, runner: CliRunner) -> None: + """apm experimental disable --verbose shows Config file.""" + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + result = runner.invoke(experimental, ["disable", "verbose-version", "-v"]) + assert result.exit_code == 0 + assert "Config file:" in result.output + + def test_reset_verbose_after_subcommand(self, runner: CliRunner) -> None: + """apm experimental reset --verbose shows Config file.""" + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["reset", "--verbose"]) + assert result.exit_code == 0 + assert "Config file:" in result.output + + +# --------------------------------------------------------------------------- +# --json output (P-UX-2) +# --------------------------------------------------------------------------- + + +class TestJsonOutput: + """Tests for `apm experimental list --json`.""" + + def test_json_output_parses_and_has_correct_schema(self, runner: CliRunner) -> None: + """--json outputs valid JSON with the expected keys for each flag.""" + import json + + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--json"]) + assert result.exit_code == 0 + + rows = json.loads(result.output) + assert isinstance(rows, list) + assert len(rows) >= 1 + + for row in rows: + assert set(row.keys()) == {"name", "enabled", "default", "description", "source"} + assert isinstance(row["name"], str) + assert isinstance(row["enabled"], bool) + assert isinstance(row["default"], bool) + assert isinstance(row["description"], str) + assert row["source"] in ("default", "config") + + def test_json_output_shows_default_source_when_no_override(self, runner: CliRunner) -> None: + """Without overrides, source is 'default'.""" + import json + + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--json"]) + rows = json.loads(result.output) + + vv = [r for r in rows if r["name"] == "verbose_version"][0] + assert vv["enabled"] is False + assert vv["source"] == "default" + + def test_json_output_shows_config_source_after_enable(self, runner: CliRunner) -> None: + """After enabling, source becomes 'config'.""" + import json + + from apm_cli.commands.experimental import experimental + + runner.invoke(experimental, ["enable", "verbose-version"]) + result = runner.invoke(experimental, ["list", "--json"]) + rows = json.loads(result.output) + + vv = [r for r in rows if r["name"] == "verbose_version"][0] + assert vv["enabled"] is True + assert vv["source"] == "config" + + def test_json_output_has_no_non_json_text(self, runner: CliRunner) -> None: + """--json must NOT emit preamble, symbols, or colours to stdout.""" + import json + + from apm_cli.commands.experimental import experimental + + result = runner.invoke(experimental, ["list", "--json"]) + # The entire output must be valid JSON (no preamble, no symbols) + parsed = json.loads(result.output) + assert isinstance(parsed, list) + # No Rich markup or status symbols + assert "[" not in result.output.split("\n")[0] or result.output.strip().startswith("[") + + +# --------------------------------------------------------------------------- +# Malformed value reset (C1) +# --------------------------------------------------------------------------- + + +class TestMalformedValueReset: + """Tests for bulk reset of malformed (non-bool) config overrides.""" + + def test_reset_cleans_malformed_string_override(self, runner: CliRunner, tmp_path) -> None: + """reset --yes removes a registered flag with a string value (e.g. 'true').""" + import json as _json + + import apm_cli.config as _conf + + from apm_cli.commands.experimental import experimental + + # Write a malformed config directly + config_dir = tmp_path / ".apm-malformed" + config_dir.mkdir() + config_file = config_dir / "config.json" + config_file.write_text( + _json.dumps({"experimental": {"verbose_version": "true"}}), + encoding="utf-8", + ) + + # Redirect config to the crafted file + import apm_cli.config as _mod + + orig_dir = _mod.CONFIG_DIR + orig_file = _mod.CONFIG_FILE + _mod.CONFIG_DIR = str(config_dir) + _mod.CONFIG_FILE = str(config_file) + _mod._config_cache = None + + try: + result = runner.invoke(experimental, ["reset", "--yes"]) + assert result.exit_code == 0 + assert "Nothing to reset" not in result.output + + data = _json.loads(config_file.read_text(encoding="utf-8")) + assert "verbose_version" not in data.get("experimental", {}) + finally: + _mod.CONFIG_DIR = orig_dir + _mod.CONFIG_FILE = orig_file + _mod._config_cache = None + + def test_reset_cleans_mixed_overrides_stale_and_malformed( + self, runner: CliRunner, tmp_path + ) -> None: + """reset --yes handles bool override + malformed value + stale key together.""" + import json as _json + + import apm_cli.config as _conf + + from apm_cli.commands.experimental import experimental + + config_dir = tmp_path / ".apm-mixed" + config_dir.mkdir() + config_file = config_dir / "config.json" + # One valid bool override, one malformed string, one stale unknown key + config_file.write_text( + _json.dumps({ + "experimental": { + "verbose_version": "true", # malformed (string, not bool) + "old_removed_flag": True, # stale (not in FLAGS) + } + }), + encoding="utf-8", + ) + + import apm_cli.config as _mod + + orig_dir = _mod.CONFIG_DIR + orig_file = _mod.CONFIG_FILE + _mod.CONFIG_DIR = str(config_dir) + _mod.CONFIG_FILE = str(config_file) + _mod._config_cache = None + + try: + result = runner.invoke(experimental, ["reset", "--yes"]) + assert result.exit_code == 0 + assert "Nothing to reset" not in result.output + assert "Reset all experimental features to defaults" in result.output + + data = _json.loads(config_file.read_text(encoding="utf-8")) + exp_section = data.get("experimental", {}) + assert exp_section == {} + finally: + _mod.CONFIG_DIR = orig_dir + _mod.CONFIG_FILE = orig_file + _mod._config_cache = None diff --git a/tests/unit/commands/test_helpers_version.py b/tests/unit/commands/test_helpers_version.py new file mode 100644 index 000000000..392245c83 --- /dev/null +++ b/tests/unit/commands/test_helpers_version.py @@ -0,0 +1,162 @@ +"""Unit tests for the verbose_version experimental branch in print_version. + +Covers three scenarios: + 1. Baseline: flag disabled -- output is the standard version string only. + 2. Enabled: flag enabled -- output adds Python, Platform, Install path lines + with 14-character left-justified labels and 2-space indentation. + 3. Graceful failure: if is_enabled raises, --version still prints the baseline + version string and exits 0 (the try/except wrapper must not swallow it). +""" + +from __future__ import annotations + +import re +from typing import Any +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + + +# --------------------------------------------------------------------------- +# Shared fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def runner() -> CliRunner: + """CliRunner -- stderr is merged into stdout by default in Click 8.""" + return CliRunner() + + +@pytest.fixture(autouse=True) +def _reset_config_cache(): + """Reset the in-process config cache before and after every test.""" + from apm_cli.config import _invalidate_config_cache + + _invalidate_config_cache() + yield + _invalidate_config_cache() + + +@pytest.fixture(autouse=True) +def _isolate_config(tmp_path, monkeypatch) -> None: + """Point config I/O at a throw-away temp directory.""" + import apm_cli.config as _conf + + config_dir = tmp_path / ".apm" + monkeypatch.setattr(_conf, "CONFIG_DIR", str(config_dir)) + monkeypatch.setattr(_conf, "CONFIG_FILE", str(config_dir / "config.json")) + monkeypatch.setattr(_conf, "_config_cache", None) + + +@pytest.fixture(autouse=True) +def _reset_helpers_console(monkeypatch) -> None: + """Reset the cached Rich console in _helpers so it is recreated fresh + inside each CliRunner invocation (pointing at the captured stdout). + """ + import apm_cli.commands._helpers as _h + + monkeypatch.setattr(_h, "_console", None) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _invoke_version(runner: CliRunner) -> Any: + """Invoke `apm --version` with update-check and experimental imports isolated.""" + from apm_cli.cli import cli + + with patch("apm_cli.commands._helpers._check_and_notify_updates"): + return runner.invoke(cli, ["--version"]) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +class TestPrintVersionVerboseVersionFlag: + """Tests for the experimental verbose_version branch in print_version.""" + + def test_baseline_output_when_flag_disabled( + self, runner: CliRunner, monkeypatch + ) -> None: + """Standard --version output when verbose_version is disabled (default).""" + import apm_cli.config as _conf + + # Explicitly no experimental override -- flag stays at default (False). + monkeypatch.setattr(_conf, "_config_cache", {}) + + result = _invoke_version(runner) + + assert result.exit_code == 0 + # Baseline string must contain the CLI name or version label. + assert "APM" in result.output or "version" in result.output.lower() + # Verbose fields must NOT appear. + assert "Python:" not in result.output + assert "Platform:" not in result.output + assert "Install path:" not in result.output + + def test_verbose_version_enabled_adds_python_platform_installpath( + self, runner: CliRunner, monkeypatch + ) -> None: + """With verbose_version=True, three labelled lines appear after the version.""" + import apm_cli.config as _conf + + monkeypatch.setattr( + _conf, + "_config_cache", + {"experimental": {"verbose_version": True}}, + ) + + result = _invoke_version(runner) + + assert result.exit_code == 0 + + # All three label lines must be present. + assert "Python:" in result.output + assert "Platform:" in result.output + assert "Install path:" in result.output + + # Each label must be left-justified in a 14-character field and indented + # with two leading spaces. Pattern: "