diff --git a/CHANGELOG.md b/CHANGELOG.md index c14c14ced..f78dc5011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Breaking Changes + +- The CLI self-updater moved from `apm update` to `apm self-update`. Inside an `apm.yml` project the bare `apm update` verb now refreshes project dependencies (matching `npm update`, `uv lock --upgrade`, `cargo update`); outside a project it forwards to `apm self-update` with a deprecation banner for one release. CI scripts that called `apm update` to refresh the binary should migrate now: `sed -i 's/apm update/apm self-update/g' your_scripts`. (#1244) + ### Added +- `apm update` now does what every npm/pip/cargo user expects -- resolve latest refs, show a structured plan, and ask before touching anything. Refreshes APM dependencies in the current project: resolves `apm.yml` against the latest refs, prints a structured plan (added/updated/removed/unchanged) with an inline legend, and prompts `[y/N]` before mutating anything (`--yes` skips the prompt; `--dry-run` previews). (#1244) +- `apm self-update`: updates the APM CLI binary itself (or shows distributor guidance when self-update is disabled at build time). Replaces the former `apm update` self-update path; supports `--check` to only check for a newer version. (#1244) +- `apm install --frozen` performs a CI-safe, read-only install that fails fast (exit 1) when `apm.lock.yaml` is missing or out of sync with `apm.yml`; mutually exclusive with `--update`. Structural presence check only; use `apm audit` for on-disk SHA integrity. (#1244) +- `apm install` now emits a one-line "Run 'apm update' to check for newer versions." hint on the no-op path when a lockfile is already present, pointing users at the verb that actually checks for newer refs. (#1244) - Virtual subdirectory and raw-file packages now resolve from self-hosted Git services (Gitea, Gogs) via raw URL with API v1/v3 fallback. (#587) - `shared/apm.md` gh-aw shared workflow exposes a `target:` import input (default `all`) so consumer workflows can ship slim, single-harness bundles instead of always packing every layout. (#1184) - **GitLab host support:** `gitlab.com` and self-managed instances (via `GITLAB_HOST` / `APM_GITLAB_HOSTS`) use GitLab REST **v4** for `marketplace.json` and install-time raw file reads; nested GitLab group paths are disambiguated in dependency references with object-form `git:` + `path:` where shorthand is ambiguous. GitHub, GHES, Azure DevOps, and registry-proxy behavior remain unchanged. (#1149) @@ -17,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- `apm install --force` help text now states explicitly that the flag does NOT refresh refs; users who want newer commits should run `apm update`. (#1244) +- `apm update` now walks parent directories to find `apm.yml` (matching npm/cargo ergonomics), emits a one-time CI banner when invoked from a project root under `CI`/`GITHUB_ACTIONS` so pipelines see the dependency-refresh shift, and the no-op nudge copy now reads "Lockfile already satisfied -- run `apm update` to resolve latest refs." `apm install --frozen` adds a post-success "Run `apm audit` for on-disk content integrity." footer to set the right expectation, and FrozenInstallError now renders identically across `install` and `update`. (#1244) - `apm marketplace browse/search/add/update` route through the registry proxy when `PROXY_REGISTRY_URL` is set; `PROXY_REGISTRY_ONLY=1` blocks direct GitHub and GitLab host API fallbacks. (#1149) - Registry proxy now warns when `PROXY_REGISTRY_TOKEN` is set and `PROXY_REGISTRY_URL` uses `http://`, since the bearer token would be transmitted in plaintext; set `PROXY_REGISTRY_ALLOW_HTTP=1` to silence the warning for trusted internal proxies. (#1149) - Integration tests now use marker-driven discovery: 21 `pytestmark = pytest.mark.skipif(...)` chains across `tests/integration/` are replaced with declarative `requires_*` markers, with precondition logic centralized in `tests/integration/conftest.py` and auto-skipping at collection time. PR1 of #1166. (#1167) diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index b4c1fbd8d..3f224a22d 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -325,6 +325,44 @@ apm compile # Instructions with matching applyTo patterns are merged from all sources ``` +## Updating dependencies + +`apm update` refreshes the APM packages declared in your `apm.yml` to their latest matching refs. It resolves the dependency graph against the network, prints a structured plan (added / updated / removed), and prompts before mutating anything. + +```bash +# Interactive: resolve, show plan, prompt [y/N], then install +apm update + +# Show the plan and exit -- no on-disk changes +apm update --dry-run + +# CI-safe: skip the prompt +apm update --yes +``` + +The plan output uses the standard bracket symbols (`[~]` updated, `[+]` added, `[-]` removed) and includes an inline legend in the footer. When nothing has changed, `apm update` prints `All dependencies already at their latest matching refs.` and exits cleanly. + +For lockfile-only enforcement in CI, pair `apm update` (in your branch / PR) with `apm install --frozen` (in CI): + +```bash +# In CI -- exit 1 if apm.lock.yaml is missing or out of sync with apm.yml +apm install --frozen +``` + +:::note[--frozen scope] +`apm install --frozen` is a **structural presence check**: it verifies every direct dependency in `apm.yml` has a lock entry. It does NOT verify that on-disk content matches the locked SHA. Use `apm audit` for end-to-end integrity verification of deployed files. +::: + +:::caution[Breaking change: `apm update` now refreshes dependencies] +In earlier releases, `apm update` updated the APM CLI binary. That behaviour moved to `apm self-update`. Inside an `apm.yml` project, the bare `apm update` verb now refreshes project dependencies (matching `npm update`, `cargo update`, `uv lock --upgrade`). + +Outside a project, `apm update` still forwards to `apm self-update` with a deprecation banner for one release. CI scripts that called `apm update` to refresh the binary should migrate now: + +```bash +sed -i 's/apm update/apm self-update/g' your_scripts +``` +::: + ## Development Dependencies Some packages are only needed during authoring — test fixtures, linting rules, internal helpers. Install them as dev dependencies so they stay out of distributed bundles: diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index 5eff49b25..8c619aaff 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -53,7 +53,9 @@ The lock file serves four goals: |-------|----------------------| | `apm install` (first run) | Created. All dependencies resolved, commits pinned, files recorded. | | `apm install` (subsequent) | Read. Locked commits reused. New dependencies appended. File only written when semantic content changes (dependencies, MCP servers/configs, `lockfile_version`); no churn from `generated_at` or `apm_version` fields. | -| `apm install --update` | Re-resolved. All refs re-resolved to latest matching commits. | +| `apm install --frozen` | Read-only. Fails fast (exit 1) if the lockfile is missing or any direct dependency in `apm.yml` is absent from the lockfile. Mutually exclusive with `--update`. Use in CI to catch drift between manifest and lockfile. | +| `apm update` | Re-resolved with confirmation gate. Resolves `apm.yml` against the latest matching refs, prints a structured plan (added/updated/removed/unchanged), and writes only after the user confirms (default `[y/N]`; bypass with `--yes`, preview with `--dry-run`). | +| `apm install --update` | Re-resolved. All refs re-resolved to latest matching commits without a confirmation prompt. Prefer `apm update` for interactive flows. | | `apm deps update` | Re-resolved. Refreshes versions for specified or all dependencies. | | `apm pack` | Enriched. A `pack:` section is prepended to the bundled copy (see [section 6](#6-pack-enrichment)). Both `--format plugin` (default) and `--format apm` embed the enriched copy when a project lockfile exists. | | `apm uninstall` | Updated. Removed dependency entries and their `deployed_files` references. | @@ -261,6 +263,19 @@ The dependency resolver interacts with the lock file as follows: to their latest commits. If a resolved commit matches the existing lock file entry and the local checkout is intact, the download is skipped. Otherwise, the package is re-fetched. The lock file is always refreshed. +4. **Interactive update** (`apm update`) -- re-resolve all refs and render + a structured plan (added/updated/removed/unchanged) before any + mutation. Defaults to a confirmation prompt; `--dry-run` exits after + the plan with no on-disk changes; `--yes` skips the prompt for CI + automation. On confirmation, behaves like (3) and refreshes the lock + file. +5. **Frozen** (`apm install --frozen`) -- read `apm.lock.yaml` and + structurally verify every direct dependency declared in `apm.yml` + has a corresponding lock entry. Exit code 1 when the lockfile is + missing or any direct dep is unlocked; never resolves, never + mutates. Mutually exclusive with `--update`. Note: this is a + structural presence check; on-disk SHA integrity is the job of + `apm audit`. When a locked commit is no longer reachable (force-pushed branch, deleted tag), APM MUST report an error and refuse to install until the lock file is updated. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 20bc18fa1..acf4a3349 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -10,7 +10,7 @@ | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` refresh refs, `--force` overwrite, `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global, `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from SKILL_BUNDLE (repeatable; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry, `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry | +| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global, `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from SKILL_BUNDLE (repeatable; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry, `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry | | `apm targets` | Show resolved deployment targets for the current project (Click group; reads filesystem signals; works with or without `apm.yml`) | `--all` also include the `agent-skills` meta-target (only meaningful with `--json`), `--json` machine-readable output. No provenance line is printed (the table is the provenance). | | `apm uninstall PKGS...` | Remove packages | `--dry-run`, `-g` global | | `apm prune` | Remove orphaned packages | `--dry-run` | @@ -161,6 +161,7 @@ Experimental flags MUST NOT gate security-critical behaviour (content scanning, | `apm config get [KEY]` | Get a config value (`auto-integrate`, `temp-dir`, `copilot-cowork-skills-dir`) | -- | | `apm config set KEY VALUE` | Set a config value (`auto-integrate`, `temp-dir`; `copilot-cowork-skills-dir` requires `apm experimental enable copilot-cowork`) | -- | | `apm config unset KEY` | Remove a stored config value (`temp-dir`, `copilot-cowork-skills-dir`) | -- | -| `apm update` | Update APM itself (or show distributor guidance when self-update is disabled at build time) | `--check` only check | +| `apm update` | Refresh APM dependencies in the current project: resolves `apm.yml` against the latest refs, prints a structured plan (added/updated/removed/unchanged), and prompts before mutating anything (default `[y/N]`). Skips the prompt with `--yes`; previews without changes with `--dry-run`. | `--yes`, `--dry-run`, `--verbose` | +| `apm self-update` | Update the APM CLI itself (or show distributor guidance when self-update is disabled at build time). | `--check` only check | `apm config set copilot-cowork-skills-dir ` persists the Cowork skills directory across shells. `apm config get copilot-cowork-skills-dir` and `apm config unset copilot-cowork-skills-dir` remain available even when the `copilot-cowork` flag is disabled so leftover state can still be inspected or cleared. In `apm config` and bare `apm config get`, the `copilot-cowork-skills-dir` entry is shown only when the `copilot-cowork` flag is enabled. diff --git a/src/apm_cli/cli.py b/src/apm_cli/cli.py index 8a3c85ef6..1179090e0 100644 --- a/src/apm_cli/cli.py +++ b/src/apm_cli/cli.py @@ -35,6 +35,7 @@ from apm_cli.commands.prune import prune from apm_cli.commands.run import preview, run from apm_cli.commands.runtime import runtime +from apm_cli.commands.self_update import self_update from apm_cli.commands.targets import targets from apm_cli.commands.uninstall import uninstall from apm_cli.commands.update import update @@ -90,6 +91,7 @@ def cli(ctx): cli.add_command(uninstall) cli.add_command(prune) cli.add_command(update) +cli.add_command(self_update) cli.add_command(compile_cmd, name="compile") cli.add_command(run) cli.add_command(preview) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 69a4d017e..ab0d7f5d5 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -6,14 +6,23 @@ import os import sys import time +from collections.abc import Callable from pathlib import Path -from typing import Any, Optional +from typing import TYPE_CHECKING, Any, Optional import click -from apm_cli.install.errors import AuthenticationError, DirectDependencyError, PolicyViolationError +from apm_cli.install.errors import ( + AuthenticationError, + DirectDependencyError, + FrozenInstallError, + PolicyViolationError, +) from apm_cli.install.gitlab_resolver import _try_resolve_gitlab_direct_shorthand +if TYPE_CHECKING: + from apm_cli.install.plan import UpdatePlan + # Re-export the pre-deploy security scan so that bare-name call sites inside # this module and ``tests/unit/test_install_scanning.py``'s direct import # (``from apm_cli.commands.install import _pre_deploy_security_scan``) keep @@ -101,7 +110,7 @@ from ..install.mcp.registry import ( validate_registry_url as _validate_registry_url, ) -from ..utils.console import _rich_echo, _rich_error, _rich_success # noqa: F401 +from ..utils.console import _rich_echo, _rich_error, _rich_info, _rich_success # noqa: F401 from ._helpers import ( _create_minimal_apm_yml, _get_default_config, @@ -215,6 +224,8 @@ class InstallContext: manifest_snapshot: bytes | None = None snapshot_manifest_path: Optional["Path"] = None legacy_skill_paths: bool = False + frozen: bool = False + plan_callback: "Callable[[UpdatePlan], bool] | None" = None # --------------------------------------------------------------------------- @@ -833,12 +844,21 @@ def _handle_mcp_install( type=click.Choice(["apm", "mcp"]), help="Install only specific dependency type", ) -@click.option("--update", is_flag=True, help="Update dependencies to latest Git references") +@click.option( + "--update", + is_flag=True, + help="Update dependencies to latest Git references (deprecated: prefer 'apm update' for an interactive plan, or 'apm update --yes' for CI)", +) @click.option("--dry-run", is_flag=True, help="Show what would be installed without installing") @click.option( "--force", is_flag=True, - help="Overwrite locally-authored files on collision and deploy despite critical security findings", + help="Overwrite locally-authored files on collision and deploy despite critical security findings (does NOT refresh refs; use 'apm update' for that)", +) +@click.option( + "--frozen", + is_flag=True, + help="Refuse to install when apm.lock.yaml is missing or out of sync with apm.yml (CI-safe; mutually exclusive with --update). Structural presence check only; use 'apm audit' for on-disk integrity.", ) @click.option("--verbose", "-v", is_flag=True, help="Show detailed installation information") @click.option( @@ -1015,6 +1035,7 @@ def install( # noqa: PLR0913 update, dry_run, force, + frozen, verbose, trust_transitive_mcp, parallel_downloads, @@ -1076,6 +1097,11 @@ def install( # noqa: PLR0913 install_started_at = time.perf_counter() summary_rendered = False logger = None + if frozen and update: + raise click.UsageError( + "--frozen and --update are mutually exclusive. " + "Use 'apm update' to refresh refs, then 'apm install --frozen' in CI." + ) try: # Create structured logger for install output early so exception # handlers can always reference it (avoids UnboundLocalError if @@ -1401,6 +1427,8 @@ def install( # noqa: PLR0913 manifest_snapshot=_manifest_snapshot, snapshot_manifest_path=_snapshot_manifest_path, legacy_skill_paths=legacy_skill_paths, + frozen=frozen, + plan_callback=None, ) apm_count, mcp_count, apm_diagnostics = _install_apm_packages( @@ -1418,6 +1446,17 @@ def install( # noqa: PLR0913 ) summary_rendered = True + if frozen and apm_count > 0: + # --frozen verifies LOCKFILE STRUCTURE (every apm.yml dep + # has a lock entry), not on-disk content integrity. Make + # the scope explicit so a CI pipeline that skips + # 'apm audit' on the assumption that --frozen covers SHA + # verification is corrected at the moment of use. + _rich_info( + "Lockfile presence verified. Run 'apm audit' for on-disk content integrity.", + symbol="info", + ) + except InsecureDependencyPolicyError: _maybe_rollback_manifest(_snapshot_manifest_path, _manifest_snapshot, logger) sys.exit(1) @@ -1615,6 +1654,8 @@ def _install_apm_packages(ctx, outcome): allow_protocol_fallback=ctx.allow_protocol_fallback, no_policy=ctx.no_policy, legacy_skill_paths=ctx.legacy_skill_paths, + frozen=ctx.frozen, + plan_callback=ctx.plan_callback, ) apm_count = install_result.installed_count apm_diagnostics = install_result.diagnostics @@ -1628,6 +1669,12 @@ def _install_apm_packages(ctx, outcome): if e.diagnostic_context: _rich_echo(e.diagnostic_context) sys.exit(1) + except FrozenInstallError as e: + _maybe_rollback_manifest(ctx.snapshot_manifest_path, ctx.manifest_snapshot, logger) + _rich_error(str(e)) + for reason in e.reasons: + _rich_echo(reason) + sys.exit(1) except Exception as e: _maybe_rollback_manifest(ctx.snapshot_manifest_path, ctx.manifest_snapshot, logger) # #832: surface PolicyViolationError verbatim (no double-nesting). @@ -1826,6 +1873,8 @@ def _install_apm_dependencies( # noqa: PLR0913 skill_subset: "builtins.tuple | None" = None, skill_subset_from_cli: bool = False, legacy_skill_paths: bool = False, + frozen: bool = False, + plan_callback=None, ): """Thin wrapper -- builds an :class:`InstallRequest` and delegates to :class:`apm_cli.install.service.InstallService`. @@ -1861,5 +1910,7 @@ def _install_apm_dependencies( # noqa: PLR0913 skill_subset=skill_subset, skill_subset_from_cli=skill_subset_from_cli, legacy_skill_paths=legacy_skill_paths, + frozen=frozen, + plan_callback=plan_callback, ) return InstallService().run(request) diff --git a/src/apm_cli/commands/self_update.py b/src/apm_cli/commands/self_update.py new file mode 100644 index 000000000..37a2b32ad --- /dev/null +++ b/src/apm_cli/commands/self_update.py @@ -0,0 +1,190 @@ +"""APM self-update command (renamed from ``update`` in #1203). + +This is the platform-aware self-updater for the APM CLI binary itself. +The command is exposed as ``apm self-update`` to free the ``apm update`` +verb for dependency-graph refresh (the package-manager convention). + +A back-compat shim in :mod:`apm_cli.cli` keeps ``apm update`` working as +a deprecated alias when invoked outside an APM project (no ``apm.yml`` +in the current directory). +""" + +import os +import shutil +import sys + +import click + +from ..core.command_logger import CommandLogger +from ..update_policy import get_self_update_disabled_message, is_self_update_enabled +from ..utils.subprocess_env import external_process_env +from ..version import get_version + + +def _is_windows_platform() -> bool: + """Return True when running on native Windows.""" + return sys.platform == "win32" + + +def _get_update_installer_url() -> str: + """Return the official installer URL for the current platform.""" + return "https://aka.ms/apm-windows" if _is_windows_platform() else "https://aka.ms/apm-unix" + + +def _get_update_installer_suffix() -> str: + """Return the file suffix for the downloaded installer script.""" + return ".ps1" if _is_windows_platform() else ".sh" + + +def _get_manual_update_command() -> str: + """Return the manual update command for the current platform.""" + if _is_windows_platform(): + return 'powershell -ExecutionPolicy Bypass -c "irm https://aka.ms/apm-windows | iex"' + return "curl -sSL https://aka.ms/apm-unix | sh" + + +def _get_installer_run_command(script_path: str) -> list[str]: + """Return the installer execution command for the current platform.""" + if _is_windows_platform(): + powershell_path = shutil.which("powershell") or shutil.which("pwsh") + if not powershell_path: + raise FileNotFoundError("PowerShell executable not found in PATH") + return [powershell_path, "-ExecutionPolicy", "Bypass", "-File", script_path] + + shell_path = "/bin/sh" if os.path.exists("/bin/sh") else "sh" + return [shell_path, script_path] + + +@click.command(name="self-update", help="Update the APM CLI binary itself to the latest version") +@click.option("--check", is_flag=True, help="Only check for updates without installing") +def self_update(check): + """Update APM CLI to the latest version (like npm update -g npm). + + This command fetches and installs the latest version of APM using the + official install script. It will detect your platform and architecture + automatically. + + Examples: + apm self-update # Update to latest version + apm self-update --check # Only check if update is available + """ + try: + import subprocess + import tempfile + + logger = CommandLogger("self-update") + + if not is_self_update_enabled(): + logger.warning(get_self_update_disabled_message()) + return + + current_version = get_version() + + # Skip check for development versions + if current_version == "unknown": + logger.warning("Cannot determine current version. Running in development mode?") + if not check: + logger.progress("To update, reinstall from the repository.") + return + + logger.progress(f"Current version: {current_version}") + logger.start("Checking for updates...") + + # Check for latest version + from ..utils.version_checker import get_latest_version_from_github + + latest_version = get_latest_version_from_github() + + if not latest_version: + logger.error("Unable to fetch latest version from GitHub") + logger.progress("Please check your internet connection or try again later") + sys.exit(1) + + from ..utils.version_checker import is_newer_version + + if not is_newer_version(current_version, latest_version): + logger.success( + f"You're already on the latest version: {current_version}", + symbol="check", + ) + return + + logger.progress(f"Latest version available: {latest_version}", symbol="sparkles") + + if check: + logger.warning(f"Update available: {current_version} -> {latest_version}") + logger.progress("Run 'apm self-update' (without --check) to install") + return + + # Proceed with update + logger.start("Downloading and installing update...") + + # Download install script to temp file + try: + import requests + + install_script_url = _get_update_installer_url() + response = requests.get(install_script_url, timeout=10) + response.raise_for_status() + + # Create temporary file for install script + from ..config import get_apm_temp_dir + + with tempfile.NamedTemporaryFile( + mode="w", + suffix=_get_update_installer_suffix(), + delete=False, + dir=get_apm_temp_dir(), + ) as f: + temp_script = f.name + f.write(response.text) + + if not _is_windows_platform(): + os.chmod(temp_script, 0o755) # noqa: S103 + + # Run install script + logger.progress("Running installer...", symbol="gear") + + # Note: We don't capture output so the installer can prompt when needed. + # Sanitise the environment so the installer (and the system binaries + # it spawns -- curl, tar, sudo) do not inherit the PyInstaller + # bootloader's LD_LIBRARY_PATH / DYLD_* overrides, which would + # otherwise redirect system linkers at this binary's bundled + # _internal directory. See issue #894. + result = subprocess.run( + _get_installer_run_command(temp_script), + check=False, + env=external_process_env(), + ) + + # Clean up temp file + try: # noqa: SIM105 + os.unlink(temp_script) + except Exception: + # Non-fatal: failed to delete temp install script + pass + + if result.returncode == 0: + logger.success( + f"Successfully updated to version {latest_version}!", + ) + logger.progress("Please restart your terminal or run 'apm --version' to verify") + else: + logger.error("Installation failed - see output above for details") + sys.exit(1) + + except ImportError: + logger.error("'requests' library not available") + logger.progress("Please update manually using:") + click.echo(f" {_get_manual_update_command()}") + sys.exit(1) + except Exception as e: + logger.error(f"Update failed: {e}") + logger.progress("Please update manually using:") + click.echo(f" {_get_manual_update_command()}") + sys.exit(1) + + except Exception as e: + _logger = CommandLogger("self-update") + _logger.error(f"Error during update: {e}") + sys.exit(1) diff --git a/src/apm_cli/commands/update.py b/src/apm_cli/commands/update.py index f3a0b8de4..4d66e2aa0 100644 --- a/src/apm_cli/commands/update.py +++ b/src/apm_cli/commands/update.py @@ -1,181 +1,319 @@ -"""APM update command.""" +"""``apm update`` -- refresh APM dependencies to the latest matching refs. + +This is the package-manager convention popularised by ``cargo update``, +``poetry update``, ``bundle update``, and ``npm update`` -- the verb is +about the dependency graph, not about updating the CLI binary itself. +The CLI self-updater lives at ``apm self-update`` (see +:mod:`apm_cli.commands.self_update`); when this command runs outside an +``apm.yml`` project it forwards to the self-updater as a deprecated +back-compat shim for one release (see ``update()`` below). + +What it does +------------ +``apm update`` is conceptually equivalent to ``apm install --update`` +**plus** an interactive plan-and-confirm gate: + +1. Run resolve to discover which deps would change. +2. Render a structured plan (``[~]`` updated, ``[+]`` added, + ``[-]`` removed) that names every dep, the ref/SHA transition, and + the deployed files at risk. +3. Prompt ``Apply these changes? [y/N]`` -- default **No**, mirroring + the security framing in the public response on issue #1203. +4. On ``y``: continue the install pipeline (download + integrate + + lockfile rewrite). On ``N`` / ``--dry-run`` / no-TTY: exit cleanly + with no on-disk mutations. + +Flags +----- +* ``--yes``/``-y`` -- skip the prompt (CI / automation). +* ``--dry-run`` -- render the plan and exit without prompting. +* ``--verbose``/``-v`` -- show unchanged deps in the plan and pipeline + diagnostics. + +Other ``apm install`` flags are NOT mirrored here on purpose -- the +update command stays focused on the refresh-and-confirm loop. +``apm install --update`` remains the swiss-army-knife escape hatch. +""" + +from __future__ import annotations -import os -import shutil import sys +from pathlib import Path import click -from ..core.command_logger import CommandLogger -from ..update_policy import get_self_update_disabled_message, is_self_update_enabled -from ..utils.subprocess_env import external_process_env -from ..version import get_version - - -def _is_windows_platform() -> bool: - """Return True when running on native Windows.""" - return sys.platform == "win32" - - -def _get_update_installer_url() -> str: - """Return the official installer URL for the current platform.""" - return "https://aka.ms/apm-windows" if _is_windows_platform() else "https://aka.ms/apm-unix" - - -def _get_update_installer_suffix() -> str: - """Return the file suffix for the downloaded installer script.""" - return ".ps1" if _is_windows_platform() else ".sh" - - -def _get_manual_update_command() -> str: - """Return the manual update command for the current platform.""" - if _is_windows_platform(): - return 'powershell -ExecutionPolicy Bypass -c "irm https://aka.ms/apm-windows | iex"' - return "curl -sSL https://aka.ms/apm-unix | sh" - - -def _get_installer_run_command(script_path: str) -> list[str]: - """Return the installer execution command for the current platform.""" - if _is_windows_platform(): - powershell_path = shutil.which("powershell") or shutil.which("pwsh") - if not powershell_path: - raise FileNotFoundError("PowerShell executable not found in PATH") - return [powershell_path, "-ExecutionPolicy", "Bypass", "-File", script_path] - - shell_path = "/bin/sh" if os.path.exists("/bin/sh") else "sh" - return [shell_path, script_path] +from ..core.command_logger import InstallLogger +from ..install.errors import ( + AuthenticationError, + DirectDependencyError, + FrozenInstallError, + PolicyViolationError, +) +from ..install.plan import UpdatePlan, render_plan_text +from ..utils.console import _rich_echo, _rich_error, _rich_info, _rich_success, _rich_warning + + +def _find_apm_yml(start: Path | None = None) -> Path | None: + """Walk parent directories from ``start`` (or cwd) to find ``apm.yml``. + + Matches the npm / cargo / poetry ergonomic: a developer running + ``apm update`` from a subdirectory of their project (``src/``, + ``docs/``, ``scripts/``) finds the manifest and operates on it, + rather than getting silently misrouted to the deprecated + self-update shim. + + The walk stops at the filesystem root or when an ``apm.yml`` is + found, whichever comes first. Returns the absolute path to the + ``apm.yml`` file when found; ``None`` when no project root is + discoverable from ``start`` upward. + """ + cwd = (start or Path.cwd()).resolve() + for candidate in (cwd, *cwd.parents): + manifest = candidate / "apm.yml" + if manifest.is_file(): + return manifest + return None -@click.command(help="Update APM to the latest version") -@click.option("--check", is_flag=True, help="Only check for updates without installing") -def update(check): - """Update APM CLI to the latest version (like npm update -g npm). +def _stdin_is_tty() -> bool: + """Return True only when stdin is connected to a real terminal. - This command fetches and installs the latest version of APM using the - official install script. It will detect your platform and architecture - automatically. + A non-TTY stdin (CI, piped, redirected) means we cannot safely + prompt for confirmation -- ``apm update`` aborts with guidance to + re-run with ``--yes``. + """ + try: + return sys.stdin is not None and sys.stdin.isatty() + except (AttributeError, ValueError): + return False + + +@click.command( + name="update", + help="Refresh APM dependencies to the latest matching refs", +) +@click.option( + "--yes", + "-y", + "assume_yes", + is_flag=True, + default=False, + help="Skip the confirmation prompt (for CI / automation)", +) +@click.option( + "--dry-run", + is_flag=True, + default=False, + help="Render the update plan and exit without changing anything", +) +@click.option( + "--verbose", + "-v", + is_flag=True, + default=False, + help="Show unchanged deps and detailed pipeline diagnostics", +) +@click.option( + "--check", + "check_only", + is_flag=True, + default=False, + help="(Deprecated) Forwarded to 'apm self-update --check' when run outside an apm.yml project; rejected inside a project.", + hidden=True, +) +@click.pass_context +def update( + ctx: click.Context, + assume_yes: bool, + dry_run: bool, + verbose: bool, + check_only: bool, +) -> None: + """Refresh APM dependencies to the latest matching refs. Examples: - apm update # Update to latest version - apm update --check # Only check if update is available + apm update # Resolve, show plan, prompt, then install + apm update --dry-run # Show plan only, do not change anything + apm update --yes # Skip the prompt (CI-safe) + apm update --verbose # Include unchanged deps in the plan """ + manifest_path = _find_apm_yml() + if manifest_path is None: + # Back-compat shim (one-release): when run outside a project, + # forward to the renamed self-updater so existing users keep + # working while we publicise ``apm self-update``. Removed in + # the release after this one. + from apm_cli.commands.self_update import self_update as _self_update_cmd + + _rich_warning( + "'apm update' refreshes APM dependencies. To update the CLI binary, " + "use 'apm self-update'. Forwarding for back-compat (deprecated).", + symbol="warning", + ) + ctx.invoke(_self_update_cmd, check=check_only) + return + + if check_only: + from apm_cli.commands.self_update import self_update as _self_update_cmd + + _rich_warning( + "'apm update --check' is the deprecated self-updater shim. " + "Use 'apm update --dry-run' to preview dependency changes, " + "or 'apm self-update --check' to check for a new CLI binary. " + "Forwarding for back-compat (deprecated).", + symbol="warning", + ) + ctx.invoke(_self_update_cmd, check=True) + return + + project_root = manifest_path.parent + if project_root != Path.cwd().resolve(): + _rich_info( + f"Using apm.yml at {manifest_path} (project root: {project_root})", + symbol="info", + ) + + _run_dep_update( + assume_yes=assume_yes, + dry_run=dry_run, + verbose=verbose, + project_root=project_root, + ) + + +def _run_dep_update( + *, + assume_yes: bool, + dry_run: bool, + verbose: bool, + project_root: Path | None = None, +) -> None: + """Core ``apm update`` flow: resolve, plan, prompt, install. + + When ``project_root`` is provided, the working directory is + switched to it before running so install pipeline paths + (``apm.yml``, ``apm.lock.yaml``, deployed primitives) resolve + against the discovered project root, not the caller's cwd. + """ + import os + + if project_root is not None and project_root != Path.cwd().resolve(): + os.chdir(project_root) + + # Surface the new semantics to CI users on every invocation: the + # interactive prompt aborts non-TTY runs anyway, but a banner up + # front prevents "why did our pipeline break overnight?" tickets + # from teams whose CI calls 'apm update' assuming it self-updates + # the CLI binary. + if os.environ.get("CI") or os.environ.get("GITHUB_ACTIONS"): + _rich_info( + "'apm update' refreshes APM dependencies. " + "Use 'apm self-update' to update the CLI binary.", + symbol="info", + ) + try: - import subprocess - import tempfile + from apm_cli.commands.install import _install_apm_dependencies # local import: heavy module + from apm_cli.core.scope import InstallScope + from apm_cli.models.apm_package import APMPackage + except ImportError as e: # pragma: no cover -- defensive + _rich_error(f"APM dependency system not available: {e}") + sys.exit(1) - logger = CommandLogger("update") + try: + apm_package = APMPackage.from_apm_yml(Path("apm.yml")) + except (FileNotFoundError, ValueError) as e: + _rich_error(f"Failed to parse apm.yml: {e}") + sys.exit(1) - if not is_self_update_enabled(): - logger.warning(get_self_update_disabled_message()) - return + if not apm_package.has_apm_dependencies() and not apm_package.get_dev_apm_dependencies(): + _rich_success("No APM dependencies declared in apm.yml -- nothing to update.") + return - current_version = get_version() + logger = InstallLogger(verbose=verbose, dry_run=dry_run, partial=False) - # Skip check for development versions - if current_version == "unknown": - logger.warning("Cannot determine current version. Running in development mode?") - if not check: - logger.progress("To update, reinstall from the repository.") - return + plan_state: dict[str, UpdatePlan | bool] = {"plan": None, "proceeded": False} - logger.progress(f"Current version: {current_version}") - logger.start("Checking for updates...") + def _plan_callback(plan: UpdatePlan) -> bool: + """Render plan, prompt, and decide whether to proceed.""" + plan_state["plan"] = plan - # Check for latest version - from ..utils.version_checker import get_latest_version_from_github + if not plan.has_changes: + _rich_success( + "All dependencies already at their latest matching refs.", + symbol="check", + ) + return False - latest_version = get_latest_version_from_github() + rendered = render_plan_text(plan, verbose=verbose) + if rendered: + _rich_echo(rendered) + _rich_echo("") - if not latest_version: - logger.error("Unable to fetch latest version from GitHub") - logger.progress("Please check your internet connection or try again later") - sys.exit(1) + if dry_run: + _rich_info( + "Dry run: no changes applied. Re-run without --dry-run to update.", + symbol="info", + ) + return False - from ..utils.version_checker import is_newer_version + if assume_yes: + plan_state["proceeded"] = True + return True - if not is_newer_version(current_version, latest_version): - logger.success( - f"You're already on the latest version: {current_version}", - symbol="check", - ) - return - - logger.progress(f"Latest version available: {latest_version}", symbol="sparkles") - - if check: - logger.warning(f"Update available: {current_version} -> {latest_version}") - logger.progress("Run 'apm update' (without --check) to install") - return - - # Proceed with update - logger.start("Downloading and installing update...") - - # Download install script to temp file - try: - import requests - - install_script_url = _get_update_installer_url() - response = requests.get(install_script_url, timeout=10) - response.raise_for_status() - - # Create temporary file for install script - from ..config import get_apm_temp_dir - - with tempfile.NamedTemporaryFile( - mode="w", - suffix=_get_update_installer_suffix(), - delete=False, - dir=get_apm_temp_dir(), - ) as f: - temp_script = f.name - f.write(response.text) - - if not _is_windows_platform(): - os.chmod(temp_script, 0o755) # noqa: S103 - - # Run install script - logger.progress("Running installer...", symbol="gear") - - # Note: We don't capture output so the installer can prompt when needed. - # Sanitise the environment so the installer (and the system binaries - # it spawns -- curl, tar, sudo) do not inherit the PyInstaller - # bootloader's LD_LIBRARY_PATH / DYLD_* overrides, which would - # otherwise redirect system linkers at this binary's bundled - # _internal directory. See issue #894. - result = subprocess.run( - _get_installer_run_command(temp_script), - check=False, - env=external_process_env(), + if not _stdin_is_tty(): + _rich_error( + "Cannot prompt for confirmation in non-interactive shell. " + "Re-run with --yes to apply, or --dry-run to preview." ) - - # Clean up temp file - try: # noqa: SIM105 - os.unlink(temp_script) - except Exception: - # Non-fatal: failed to delete temp install script - pass - - if result.returncode == 0: - logger.success( - f"Successfully updated to version {latest_version}!", - ) - logger.progress("Please restart your terminal or run 'apm --version' to verify") - else: - logger.error("Installation failed - see output above for details") - sys.exit(1) - - except ImportError: - logger.error("'requests' library not available") - logger.progress("Please update manually using:") - click.echo(f" {_get_manual_update_command()}") - sys.exit(1) - except Exception as e: - logger.error(f"Update failed: {e}") - logger.progress("Please update manually using:") - click.echo(f" {_get_manual_update_command()}") sys.exit(1) + proceed = click.confirm("Apply these changes?", default=False, show_default=True) + plan_state["proceeded"] = proceed + if not proceed: + _rich_info("No changes applied.", symbol="info") + return proceed + + try: + result = _install_apm_dependencies( + apm_package, + update_refs=True, + verbose=verbose, + scope=InstallScope.PROJECT, + logger=logger, + plan_callback=_plan_callback, + ) + except FrozenInstallError as e: + _rich_error(str(e)) + for reason in e.reasons: + _rich_echo(reason) + sys.exit(1) + except AuthenticationError as e: + _rich_error(str(e)) + if e.diagnostic_context: + _rich_echo(e.diagnostic_context) + sys.exit(1) + except (DirectDependencyError, PolicyViolationError) as e: + _rich_error(str(e)) + sys.exit(1) + except click.UsageError: + raise except Exception as e: - _logger = CommandLogger("update") - _logger.error(f"Error during update: {e}") + _rich_error(f"Error updating dependencies: {e}") + if not verbose: + _rich_info("Run with --verbose for detailed diagnostics.") sys.exit(1) + + plan = plan_state.get("plan") + if plan is None or not isinstance(plan, UpdatePlan): + return + + if plan_state.get("proceeded"): + installed = getattr(result, "installed_count", 0) + if installed: + _rich_success(f"Updated {installed} APM dependencies.") + else: + _rich_success("Update applied.") + + +__all__ = ["update"] diff --git a/src/apm_cli/core/command_logger.py b/src/apm_cli/core/command_logger.py index 7632ace47..55f3826e5 100644 --- a/src/apm_cli/core/command_logger.py +++ b/src/apm_cli/core/command_logger.py @@ -262,12 +262,30 @@ def resolution_start(self, to_install_count: int, lockfile_count: int): if lockfile_count > 0: _rich_info(f"Using apm.lock.yaml ({lockfile_count} locked dependencies)") - def nothing_to_install(self): - """Log when there's nothing to install — context-aware message.""" + def nothing_to_install( + self, + lockfile_present: bool = False, + update_mode: bool = False, + ): + """Log when there's nothing to install -- context-aware message. + + Args: + lockfile_present: True when apm.lock.yaml exists on disk at + the time of the no-op. When True (and we're not in + update mode) we append the standard hint pointing at + ``apm update`` -- this is the #1203 nudge that keeps + users from believing ``apm install`` checks for newer + versions. + update_mode: True when this run was invoked with + ``--update`` or via ``apm update``. Suppresses the + hint -- the user already asked to refresh. + """ if self.partial: _rich_info("Requested packages are already installed.", symbol="check") else: _rich_success("All dependencies are up to date.", symbol="check") + if lockfile_present and not update_mode: + _rich_info("Lockfile already satisfied -- run 'apm update' to resolve latest refs.") # --- Download phase --- diff --git a/src/apm_cli/install/errors.py b/src/apm_cli/install/errors.py index 0b418d5b0..85d7dbb9f 100644 --- a/src/apm_cli/install/errors.py +++ b/src/apm_cli/install/errors.py @@ -62,6 +62,29 @@ def __init__(self, message: str, *, diagnostic_context: str = ""): self.diagnostic_context = diagnostic_context +class FrozenInstallError(RuntimeError): + """Raised when ``apm install --frozen`` cannot proceed. + + Two trigger conditions: + + * Lockfile (``apm.lock.yaml``) is missing entirely. + * Lockfile is structurally out of sync with ``apm.yml`` -- a direct + dependency declared in the manifest has no entry in the lockfile. + In that case ``reasons`` carries one human-readable line per + missing dep so the renderer can list them. + + The check is intentionally narrow: it flags the cases where running + install without ``--frozen`` would mutate the lockfile. Drift in + transitive deps or removed deps is allowed, mirroring how ``uv`` + treats ``--frozen`` and how ``npm ci`` only enforces direct-deps + presence. + """ + + def __init__(self, message: str, *, reasons: list[str] | None = None): + super().__init__(message) + self.reasons = list(reasons or []) + + class PolicyViolationError(RuntimeError): """Raised when org-policy enforcement halts an install. diff --git a/src/apm_cli/install/pipeline.py b/src/apm_cli/install/pipeline.py index 1785ba2ca..733a16d10 100644 --- a/src/apm_cli/install/pipeline.py +++ b/src/apm_cli/install/pipeline.py @@ -264,6 +264,7 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 skill_subset: tuple | None = None, skill_subset_from_cli: bool = False, legacy_skill_paths: bool = False, + plan_callback=None, ): """Install APM package dependencies. @@ -401,9 +402,33 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 if not ctx.deps_to_install and not ctx.root_has_local_primitives and not _has_orphan_deps: if logger: - logger.nothing_to_install() + logger.nothing_to_install( + lockfile_present=_early_lockfile is not None, + update_mode=update_refs, + ) return InstallResult() + # ------------------------------------------------------------------ + # Plan-gate checkpoint (#1203): show the user what install/update + # is about to do and let them confirm. Invoked AFTER resolve so we + # have ``ctx.deps_to_install`` with resolved refs, BEFORE downloads + # begin so a "no" answer cancels cleanly without touching the + # cache. + # + # Only ``apm update`` passes a callback today; all other entry + # points pass ``None`` and the checkpoint is a no-op. The TUI is + # already exited (see the ``finally`` above), so callbacks can + # write directly to stdout / call ``click.confirm`` without + # collision. + # ------------------------------------------------------------------ + if plan_callback is not None: + from .plan import build_update_plan + + plan = build_update_plan(_early_lockfile, ctx.deps_to_install) + proceed = plan_callback(plan) + if not proceed: + return InstallResult() + ctx.tui.__enter__() try: # -------------------------------------------------------------- diff --git a/src/apm_cli/install/plan.py b/src/apm_cli/install/plan.py new file mode 100644 index 000000000..799be4197 --- /dev/null +++ b/src/apm_cli/install/plan.py @@ -0,0 +1,425 @@ +"""Update plan: structured diff between current lockfile and fresh resolution. + +This module is the support library for the ``apm update`` command (#1203). +It also provides the structural-satisfaction check used by +``apm install --frozen``. + +Two responsibilities: + +1. ``build_update_plan`` -- pure comparison of an old :class:`LockFile` + against a list of freshly-resolved :class:`DependencyReference` + objects (post-resolve, pre-download). Produces an + :class:`UpdatePlan` of immutable :class:`PlanEntry` records, each + capturing one dependency's before/after state. + +2. ``render_plan_text`` -- ASCII rendering of an :class:`UpdatePlan` + suitable for terminal display, using the bracket-status convention + from ``.github/instructions/encoding.instructions.md``. + +3. ``lockfile_satisfies_manifest`` -- structural check: does a lockfile + carry an entry for every direct dependency declared in the manifest? + Used to enforce ``--frozen`` without running the resolve phase. + +Design notes +------------ +* No I/O. No network. Every function in this module is pure -- + testable in isolation by feeding fixture lockfiles and dep lists. +* Frozen dataclasses (``PlanEntry``, ``UpdatePlan``) so callers can + freely pass them across phase boundaries without aliasing risk. +* The ``deployed_files`` shown in a plan entry are taken from the + EXISTING lockfile (i.e. what is currently on disk). We do not yet + know the post-update file list at the plan checkpoint, since + integration has not run. Showing the "files at risk" surface is + honest enough for P0; a richer post-download diff is a P1+ + enhancement. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from apm_cli.utils.console import STATUS_SYMBOLS + +if TYPE_CHECKING: + from collections.abc import Iterable + + from apm_cli.deps.lockfile import LockedDependency, LockFile + from apm_cli.models.dependency.reference import DependencyReference + + +_ACTION_UPDATE = "update" +_ACTION_ADD = "add" +_ACTION_REMOVE = "remove" +_ACTION_UNCHANGED = "unchanged" + + +def _dep_ref_key(dep: DependencyReference) -> str: + """Compute the unique key for a manifest dependency. + + Mirrors :meth:`LockedDependency.get_unique_key` so that manifest and + lockfile entries can be matched 1:1 without round-tripping through + a full resolution. + + Local refs use ``local_path``; virtual subdirectory refs use + ``repo_url/virtual_path``; everything else is keyed on ``repo_url``. + """ + if getattr(dep, "is_local", False) and dep.local_path: + return dep.local_path + if getattr(dep, "is_virtual", False) and dep.virtual_path: + return f"{dep.repo_url}/{dep.virtual_path}" + return dep.repo_url + + +def _short_sha(commit: str | None, length: int = 7) -> str: + """Render a commit SHA (or placeholder) in short form. + + Returns ``-`` when no commit is available, so the diff shows a + visible delta even on the "before" or "after" side of an + add / remove entry. + """ + if not commit: + return "-" + return commit[:length] + + +@dataclass(frozen=True) +class PlanEntry: + """One dependency's before/after state in an :class:`UpdatePlan`. + + ``action`` is one of ``"update"``, ``"add"``, ``"remove"``, or + ``"unchanged"`` -- mutually exclusive. Callers should use the + ``has_changes`` property rather than comparing strings. + + ``deployed_files`` reflects the existing lockfile only -- see module + docstring for rationale. + """ + + dep_key: str + action: str + display_name: str = "" + + old_resolved_ref: str | None = None + old_resolved_commit: str | None = None + old_content_hash: str | None = None + + new_resolved_ref: str | None = None + new_resolved_commit: str | None = None + + deployed_files: tuple[str, ...] = () + + @property + def has_changes(self) -> bool: + return self.action != _ACTION_UNCHANGED + + @property + def short_old_commit(self) -> str: + return _short_sha(self.old_resolved_commit) + + @property + def short_new_commit(self) -> str: + return _short_sha(self.new_resolved_commit) + + +@dataclass(frozen=True) +class UpdatePlan: + """Structured diff between current lockfile and a fresh resolution.""" + + entries: tuple[PlanEntry, ...] = () + + @property + def has_changes(self) -> bool: + return any(e.has_changes for e in self.entries) + + @property + def changed_entries(self) -> tuple[PlanEntry, ...]: + return tuple(e for e in self.entries if e.has_changes) + + @property + def summary_counts(self) -> dict[str, int]: + counts = { + _ACTION_UPDATE: 0, + _ACTION_ADD: 0, + _ACTION_REMOVE: 0, + _ACTION_UNCHANGED: 0, + } + for e in self.entries: + counts[e.action] = counts.get(e.action, 0) + 1 + return counts + + +def _display_name(dep_key: str, locked: LockedDependency | None) -> str: + """Pick a short, human-friendly label for a dep entry. + + Prefers ``repo_url`` (with ``virtual_path`` suffix when present) + from the locked record, since the manifest reference may be a bare + shorthand without the resolved host. + """ + if locked is not None: + name = locked.repo_url + if getattr(locked, "virtual_path", None): + name = f"{name}/{locked.virtual_path}" + return name + return dep_key + + +def build_update_plan( + old_lockfile: LockFile | None, + resolved_deps: Iterable[DependencyReference], +) -> UpdatePlan: + """Compare an existing lockfile against freshly-resolved deps. + + Args: + old_lockfile: Current on-disk lockfile, or None when the project + has never been installed before. + resolved_deps: Output of the resolve phase -- each + ``DependencyReference`` carries a ``resolved_reference`` + populated by the resolver. Typically + ``InstallContext.deps_to_install``. + + Returns: + A frozen :class:`UpdatePlan` summarising the diff. + """ + old_entries: dict[str, LockedDependency] = {} + if old_lockfile is not None: + from apm_cli.deps.lockfile import _SELF_KEY + + old_entries = { + key: dep for key, dep in old_lockfile.dependencies.items() if key != _SELF_KEY + } + + seen_keys: set[str] = set() + plan_entries: list[PlanEntry] = [] + + for dep in resolved_deps: + key = _dep_ref_key(dep) + seen_keys.add(key) + old = old_entries.get(key) + new_ref, new_commit = _extract_new_ref_and_commit(dep) + + if old is None: + plan_entries.append( + PlanEntry( + dep_key=key, + action=_ACTION_ADD, + display_name=_display_name(key, None) or dep.repo_url, + new_resolved_ref=new_ref, + new_resolved_commit=new_commit, + ) + ) + continue + + old_ref = old.resolved_ref + old_commit = old.resolved_commit + deployed = tuple(old.deployed_files) + + if (old_commit or None) == (new_commit or None) and (old_ref or None) == (new_ref or None): + plan_entries.append( + PlanEntry( + dep_key=key, + action=_ACTION_UNCHANGED, + display_name=_display_name(key, old), + old_resolved_ref=old_ref, + old_resolved_commit=old_commit, + old_content_hash=old.content_hash, + new_resolved_ref=new_ref, + new_resolved_commit=new_commit, + deployed_files=deployed, + ) + ) + continue + + plan_entries.append( + PlanEntry( + dep_key=key, + action=_ACTION_UPDATE, + display_name=_display_name(key, old), + old_resolved_ref=old_ref, + old_resolved_commit=old_commit, + old_content_hash=old.content_hash, + new_resolved_ref=new_ref, + new_resolved_commit=new_commit, + deployed_files=deployed, + ) + ) + + for key, old in old_entries.items(): + if key in seen_keys: + continue + plan_entries.append( + PlanEntry( + dep_key=key, + action=_ACTION_REMOVE, + display_name=_display_name(key, old), + old_resolved_ref=old.resolved_ref, + old_resolved_commit=old.resolved_commit, + old_content_hash=old.content_hash, + deployed_files=tuple(old.deployed_files), + ) + ) + + plan_entries.sort(key=lambda e: (_ACTION_ORDER.get(e.action, 99), e.display_name or e.dep_key)) + return UpdatePlan(entries=tuple(plan_entries)) + + +_ACTION_ORDER = { + _ACTION_UPDATE: 0, + _ACTION_ADD: 1, + _ACTION_REMOVE: 2, + _ACTION_UNCHANGED: 3, +} + + +def _extract_new_ref_and_commit(dep: DependencyReference) -> tuple[str | None, str | None]: + """Pull ``(resolved_ref, resolved_commit)`` from a resolved dep. + + ``DependencyReference`` carries an optional ``resolved_reference`` + (a :class:`ResolvedReference`) that the resolve phase populates. + Both halves are optional; either may be ``None`` if resolution did + not yield that field. + """ + resolved = getattr(dep, "resolved_reference", None) + if resolved is None: + return (getattr(dep, "reference", None), None) + new_ref = ( + getattr(resolved, "ref_name", None) + or getattr(resolved, "original_ref", None) + or getattr(dep, "reference", None) + ) + new_commit = getattr(resolved, "resolved_commit", None) + return (new_ref, new_commit) + + +def _format_ref_change(entry: PlanEntry) -> str: + """Render the ref/commit transition for a single :class:`PlanEntry`. + + Examples (ASCII only): + ``main (abc1234 -> def5678)`` + ``v1.0.0 (new)`` + ``main (abc1234 removed)`` + """ + if entry.action == _ACTION_ADD: + ref = entry.new_resolved_ref or "-" + commit = entry.short_new_commit + return f"{ref} ({commit}, new)" + if entry.action == _ACTION_REMOVE: + ref = entry.old_resolved_ref or "-" + commit = entry.short_old_commit + return f"{ref} ({commit}, removed)" + old_ref = entry.old_resolved_ref or "-" + new_ref = entry.new_resolved_ref or old_ref + ref_part = old_ref if old_ref == new_ref else f"{old_ref} -> {new_ref}" + return f"{ref_part} ({entry.short_old_commit} -> {entry.short_new_commit})" + + +def render_plan_text(plan: UpdatePlan, *, verbose: bool = False) -> str: + """Render an :class:`UpdatePlan` as ASCII terminal output. + + Empty string when ``plan.has_changes`` is False (callers display + a higher-level "already up to date" message instead). + + Bracket-status symbols: + ``[~]`` updated + ``[+]`` added + ``[-]`` removed + ``[=]`` unchanged (verbose only) + + The output never includes a trailing newline; callers append one if + needed. + """ + if not plan.has_changes and not verbose: + return "" + + lines: list[str] = ["[i] Update plan for apm.yml", ""] + for entry in plan.entries: + if entry.action == _ACTION_UNCHANGED and not verbose: + continue + symbol = _ACTION_SYMBOLS.get(entry.action, "[?]") + lines.append(f" {symbol} {entry.display_name}") + lines.append(f" ref: {_format_ref_change(entry)}") + if entry.deployed_files: + preview = ", ".join(entry.deployed_files[:3]) + if len(entry.deployed_files) > 3: + preview += f", +{len(entry.deployed_files) - 3} more" + lines.append(f" files: {preview}") + lines.append("") + + counts = plan.summary_counts + summary_parts = [] + if counts.get(_ACTION_UPDATE): + summary_parts.append(f"{counts[_ACTION_UPDATE]} updated") + if counts.get(_ACTION_ADD): + summary_parts.append(f"{counts[_ACTION_ADD]} added") + if counts.get(_ACTION_REMOVE): + summary_parts.append(f"{counts[_ACTION_REMOVE]} removed") + if verbose and counts.get(_ACTION_UNCHANGED): + summary_parts.append(f"{counts[_ACTION_UNCHANGED]} unchanged") + if summary_parts: + lines.append(" " + ", ".join(summary_parts)) + legend_bits = [] + if counts.get(_ACTION_UPDATE): + legend_bits.append(f"{_ACTION_SYMBOLS[_ACTION_UPDATE]} updated") + if counts.get(_ACTION_ADD): + legend_bits.append(f"{_ACTION_SYMBOLS[_ACTION_ADD]} added") + if counts.get(_ACTION_REMOVE): + legend_bits.append(f"{_ACTION_SYMBOLS[_ACTION_REMOVE]} removed") + if verbose and counts.get(_ACTION_UNCHANGED): + legend_bits.append(f"{_ACTION_SYMBOLS[_ACTION_UNCHANGED]} unchanged") + if legend_bits: + lines.append(" " + " ".join(legend_bits)) + + return "\n".join(lines).rstrip() + + +_ACTION_SYMBOLS = { + _ACTION_UPDATE: STATUS_SYMBOLS["update"], + _ACTION_ADD: STATUS_SYMBOLS["check"], + _ACTION_REMOVE: STATUS_SYMBOLS["remove"], + _ACTION_UNCHANGED: STATUS_SYMBOLS["equal"], +} + + +def lockfile_satisfies_manifest( + lockfile: LockFile, + manifest_deps: Iterable[DependencyReference], +) -> tuple[bool, list[str]]: + """Structural satisfaction check for ``apm install --frozen``. + + Verifies that every direct dependency declared in the manifest has + a corresponding entry in the lockfile. Does NOT perform any + resolution or compare resolved refs against the remote -- those are + ``apm update``'s job. + + Args: + lockfile: The on-disk lockfile. + manifest_deps: Direct deps from the apm.yml manifest (regular + + dev). Local deps are skipped as they have no remote ref to + satisfy. + + Returns: + ``(satisfied, reasons)`` -- ``reasons`` is a list of + human-readable strings explaining each mismatch, empty when + ``satisfied`` is True. + """ + from apm_cli.deps.lockfile import _SELF_KEY + + locked_keys = {key for key in lockfile.dependencies if key != _SELF_KEY} + + reasons: list[str] = [] + for dep in manifest_deps: + if getattr(dep, "is_local", False): + continue + key = _dep_ref_key(dep) + if key not in locked_keys: + reasons.append(f" - {key} is declared in apm.yml but missing from apm.lock.yaml") + + return (not reasons, reasons) + + +__all__ = [ + "PlanEntry", + "UpdatePlan", + "build_update_plan", + "lockfile_satisfies_manifest", + "render_plan_text", +] diff --git a/src/apm_cli/install/request.py b/src/apm_cli/install/request.py index 41b6f083a..730eb376c 100644 --- a/src/apm_cli/install/request.py +++ b/src/apm_cli/install/request.py @@ -1,6 +1,6 @@ """Typed inputs for the install pipeline (Application Service input). -Bundles the 11 kwargs previously passed to ``run_install_pipeline`` into a +Bundles the kwargs previously passed to ``run_install_pipeline`` into a single immutable record that the Click handler builds from CLI args and the ``InstallService`` consumes. This is the typed-IO companion to ``InstallResult`` (the Service output, defined in ``apm_cli.models.results``). @@ -9,12 +9,13 @@ from __future__ import annotations from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple # noqa: F401, UP035 +from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple # noqa: F401, UP035 if TYPE_CHECKING: from apm_cli.core.auth import AuthResolver from apm_cli.core.command_logger import InstallLogger from apm_cli.core.scope import InstallScope + from apm_cli.install.plan import UpdatePlan from apm_cli.models.apm_package import APMPackage @@ -45,3 +46,15 @@ class InstallRequest: skill_subset: tuple[str, ...] | None = None # --skill filter for SKILL_BUNDLE packages skill_subset_from_cli: bool = False # True when user passed --skill (even --skill '*') legacy_skill_paths: bool = False # --legacy-skill-paths / APM_LEGACY_SKILL_PATHS + + # --frozen: refuse to install if lockfile is missing or stale relative + # to apm.yml. Enforced in InstallService.run() BEFORE delegating to + # the pipeline, so the failure surfaces without running resolve. + frozen: bool = False + + # Plan-gate hook: if set, run_install_pipeline invokes this callable + # AFTER resolve completes and BEFORE downloads begin, passing the + # computed UpdatePlan. The callable returns True to proceed or + # False to abort cleanly with a "no changes applied" message. Used + # by ``apm update`` to render the plan and prompt the user. + plan_callback: Callable[[UpdatePlan], bool] | None = None diff --git a/src/apm_cli/install/service.py b/src/apm_cli/install/service.py index 82b52586c..5d9e54415 100644 --- a/src/apm_cli/install/service.py +++ b/src/apm_cli/install/service.py @@ -50,7 +50,18 @@ def run(self, request: InstallRequest) -> InstallResult: InstallNotAvailableError: if the dependency subsystem failed to import (e.g. missing optional extras). Adapters are responsible for presenting this to the user. + FrozenInstallError: when ``request.frozen`` is True and the + lockfile is missing or structurally out of sync with + ``request.apm_package``. Raised before the pipeline + runs so no resolve / download work is wasted. """ + # Enforce --frozen BEFORE invoking the pipeline. The check is + # purely structural (no network) so it must succeed or fail in + # well under a second; running it here keeps the contract simple + # for the pipeline (which never sees a `frozen` flag). + if request.frozen: + self._enforce_frozen(request) + # Local import keeps service module import-cheap and matches the # existing pipeline's lazy-import discipline. try: @@ -78,4 +89,58 @@ def run(self, request: InstallRequest) -> InstallResult: skill_subset=request.skill_subset, skill_subset_from_cli=request.skill_subset_from_cli, legacy_skill_paths=request.legacy_skill_paths, + plan_callback=request.plan_callback, ) + + @staticmethod + def _enforce_frozen(request: InstallRequest) -> None: + """Raise :class:`FrozenInstallError` if lockfile is absent or stale. + + Looks up ``apm.lock.yaml`` next to the manifest's ``apm.yml``, + loads it, and runs ``lockfile_satisfies_manifest`` against the + manifest's direct deps (regular + dev). Any miss raises with a + list of human-readable reasons the renderer can show. + """ + from pathlib import Path + + from apm_cli.deps.lockfile import LockFile + from apm_cli.install.errors import FrozenInstallError + from apm_cli.install.plan import lockfile_satisfies_manifest + + manifest_path = getattr(request.apm_package, "package_path", None) + if manifest_path is None: + project_dir = Path(".") + elif Path(manifest_path).is_file(): + project_dir = Path(manifest_path).parent + else: + project_dir = Path(manifest_path) + lockfile_path = project_dir / "apm.lock.yaml" + + if not lockfile_path.exists(): + raise FrozenInstallError( + "--frozen requires apm.lock.yaml to exist. " + "Run 'apm install' (without --frozen) or 'apm update' first.", + ) + + try: + lockfile = LockFile.read(lockfile_path) + except Exception as e: + raise FrozenInstallError( + f"--frozen could not read apm.lock.yaml: {e}", + ) from e + + if lockfile is None: + raise FrozenInstallError( + "--frozen requires apm.lock.yaml to exist. " + "Run 'apm install' (without --frozen) or 'apm update' first.", + ) + + manifest_deps = list(request.apm_package.get_apm_dependencies()) + manifest_deps.extend(request.apm_package.get_dev_apm_dependencies()) + + satisfied, reasons = lockfile_satisfies_manifest(lockfile, manifest_deps) + if not satisfied: + raise FrozenInstallError( + "--frozen: apm.lock.yaml is out of sync with apm.yml.", + reasons=reasons, + ) diff --git a/src/apm_cli/utils/console.py b/src/apm_cli/utils/console.py index 3897b5d47..d2361fc78 100644 --- a/src/apm_cli/utils/console.py +++ b/src/apm_cli/utils/console.py @@ -56,6 +56,9 @@ "plugin": "[>]", # Plugin-related operations "search": "[>]", # Search operations "download": "[>]", # Download operations + "update": "[~]", # Plan diff: dep refreshed to a new ref/commit + "remove": "[-]", # Plan diff: dep removed from manifest + "equal": "[=]", # Plan diff: dep unchanged (verbose only) } diff --git a/tests/integration/test_update_e2e.py b/tests/integration/test_update_e2e.py new file mode 100644 index 000000000..af482b598 --- /dev/null +++ b/tests/integration/test_update_e2e.py @@ -0,0 +1,310 @@ +"""End-to-end integration tests for `apm update` and `apm install --frozen`. + +Issue: https://github.com/microsoft/apm/issues/1203 (P0). + +Validates the full pipeline against a real GitHub package: + +* `apm update --dry-run` resolves, renders a plan, and writes nothing. +* `apm update --yes` after install with no manifest changes is a no-op. +* `apm install --frozen` succeeds against an in-sync lockfile. +* `apm install --frozen` exits non-zero when lockfile is missing. +* `apm install --frozen` exits non-zero when manifest declares a dep + not present in the lockfile. +* `apm install --frozen --update` is rejected as a usage error. + +Uses the real `microsoft/apm-sample-package`. Requires GITHUB_APM_PAT +or GITHUB_TOKEN. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +pytestmark = pytest.mark.skipif( + not os.environ.get("GITHUB_APM_PAT") and not os.environ.get("GITHUB_TOKEN"), + reason="GITHUB_APM_PAT or GITHUB_TOKEN required for GitHub API access", +) + + +@pytest.fixture +def apm_command(): + apm_on_path = shutil.which("apm") + if apm_on_path: + return apm_on_path + venv_apm = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" + if venv_apm.exists(): + return str(venv_apm) + return "apm" + + +@pytest.fixture +def temp_project(tmp_path): + project_dir = tmp_path / "update-test" + project_dir.mkdir() + (project_dir / ".github").mkdir() + # Per #1154, vscode/copilot target detection requires this signal file. + (project_dir / ".github" / "copilot-instructions.md").write_text("# test\n") + return project_dir + + +def _run_apm(apm_command, args, cwd, timeout=180, stdin_input=None): + return subprocess.run( + [apm_command] + args, # noqa: RUF005 + cwd=cwd, + capture_output=True, + text=True, + timeout=timeout, + input=stdin_input, + ) + + +def _write_apm_yml(project_dir: Path, apm_packages: list[str]) -> None: + config = { + "name": "update-test", + "version": "1.0.0", + "dependencies": {"apm": apm_packages, "mcp": []}, + } + (project_dir / "apm.yml").write_text( + yaml.dump(config, default_flow_style=False), encoding="utf-8" + ) + + +class TestUpdateE2E: + def test_update_dry_run_writes_nothing(self, temp_project, apm_command): + """`apm update --dry-run` prints a plan and writes no artifacts.""" + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + + result = _run_apm(apm_command, ["update", "--dry-run"], temp_project) + + assert result.returncode == 0, ( + f"Dry-run failed:\nSTDOUT: {result.stdout}\nSTDERR: {result.stderr}" + ) + assert "Dry run" in result.stdout or "plan" in result.stdout.lower() + assert not (temp_project / "apm.lock.yaml").exists() + + def test_update_after_install_no_changes_short_circuits(self, temp_project, apm_command): + """After `apm install`, a follow-up `apm update --yes` with no + manifest changes should report all-up-to-date and not fail.""" + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + + first = _run_apm(apm_command, ["install"], temp_project) + assert first.returncode == 0, ( + f"Initial install failed:\nSTDOUT: {first.stdout}\nSTDERR: {first.stderr}" + ) + assert (temp_project / "apm.lock.yaml").exists() + + result = _run_apm(apm_command, ["update", "--yes"], temp_project) + + assert result.returncode == 0, ( + f"Update failed:\nSTDOUT: {result.stdout}\nSTDERR: {result.stderr}" + ) + + +class TestFrozenE2E: + def test_frozen_succeeds_against_in_sync_lockfile(self, temp_project, apm_command): + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + + first = _run_apm(apm_command, ["install"], temp_project) + assert first.returncode == 0, first.stderr + + # Re-run with --frozen on the same manifest+lockfile. + result = _run_apm(apm_command, ["install", "--frozen"], temp_project) + + assert result.returncode == 0, ( + f"Frozen install failed on in-sync project:\n{result.stdout}\n{result.stderr}" + ) + + def test_frozen_fails_when_lockfile_missing(self, temp_project, apm_command): + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + + result = _run_apm(apm_command, ["install", "--frozen"], temp_project) + + assert result.returncode != 0 + combined = result.stdout + result.stderr + assert "frozen" in combined.lower() or "lock" in combined.lower() + + def test_frozen_fails_when_manifest_adds_undeclared_dep(self, temp_project, apm_command): + """Lockfile present but manifest gained a dep that isn't in lock -> fail.""" + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + first = _run_apm(apm_command, ["install"], temp_project) + assert first.returncode == 0, first.stderr + + _write_apm_yml( + temp_project, + ["microsoft/apm-sample-package", "microsoft/some-other-not-in-lock"], + ) + + result = _run_apm(apm_command, ["install", "--frozen"], temp_project) + + assert result.returncode != 0 + combined = result.stdout + result.stderr + assert "out of sync" in combined.lower() or "missing" in combined.lower() + + def test_frozen_with_update_is_rejected(self, temp_project, apm_command): + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + + result = _run_apm(apm_command, ["install", "--frozen", "--update"], temp_project) + + assert result.returncode != 0 + combined = result.stdout + result.stderr + assert "frozen" in combined.lower() and "update" in combined.lower() + + +class TestUpdateInteractiveDecline: + """Cover the interactive 'user declines the plan' path end-to-end. + + The unit-tier CliRunner test mocks ``click.confirm`` and exits + before the install pipeline runs. This class exercises the full + real-binary flow with a real TTY-attached subprocess so the test + catches regressions in TTY detection, stdin plumbing, and the + lockfile-untouched contract on decline. + + Skipped on Windows: ``pty.openpty`` is POSIX-only. + """ + + @pytest.mark.skipif( + sys.platform.startswith("win"), + reason="pty.openpty is POSIX-only; decline path is covered by unit tests on Windows", + ) + def test_decline_at_prompt_leaves_lockfile_untouched(self, temp_project, apm_command): + """``apm update`` with TTY stdin and 'n' answer mutates nothing. + + Sequence: + + 1. Install the sample package -> writes ``apm.lock.yaml`` + pinning the resolved commit. + 2. Mutate ``apm.yml`` to pin a new ref so the next resolve + sees a real planned change to confirm. + 3. Spawn ``apm update`` with a real PTY attached to stdin so + the prompt actually fires. + 4. Send ``n\\n`` to decline. + 5. Assert: rc == 0, "No changes applied" surfaces, and the + lockfile commit field is byte-identical to the pre-update + state (the decline path performed no on-disk mutation). + """ + _write_apm_yml(temp_project, ["microsoft/apm-sample-package"]) + first = _run_apm(apm_command, ["install"], temp_project) + assert first.returncode == 0, first.stderr + + lockfile = temp_project / "apm.lock.yaml" + assert lockfile.exists(), "install should have produced apm.lock.yaml" + lockfile_before = lockfile.read_bytes() + + _write_apm_yml_with_ref( + temp_project, + "microsoft/apm-sample-package", + ref="main", + ) + + rc, output = _run_apm_with_pty( + apm_command, + ["update"], + cwd=temp_project, + stdin_text="n\n", + timeout=180, + ) + + assert rc == 0, f"apm update (declined) returned rc={rc}\noutput:\n{output}" + assert "no changes applied" in output.lower(), ( + f"Decline path did not surface 'No changes applied':\n{output}" + ) + lockfile_after = lockfile.read_bytes() + assert lockfile_after == lockfile_before, ( + "Lockfile bytes changed after user declined the plan -- " + "the decline path leaked an on-disk mutation." + ) + + +def _write_apm_yml_with_ref(project_dir: Path, package: str, *, ref: str) -> None: + """Write apm.yml using the structured object form so a ref pin is honored.""" + config = { + "name": "update-test", + "version": "1.0.0", + "dependencies": { + "apm": [{"repo": package, "ref": ref}], + "mcp": [], + }, + } + (project_dir / "apm.yml").write_text( + yaml.dump(config, default_flow_style=False), encoding="utf-8" + ) + + +def _run_apm_with_pty( + apm_command: str, + args: list[str], + *, + cwd: Path, + stdin_text: str, + timeout: int, +) -> tuple[int, str]: + """Run apm with a real PTY attached so interactive prompts actually fire. + + Returns ``(returncode, combined_output)``. The PTY is allocated in + the parent; stdout / stderr are merged into the master fd just like + a real terminal would surface them. ``stdin_text`` is delivered as + typed keystrokes (write to master). + """ + import os + import pty + import select + import time + + master_fd, slave_fd = pty.openpty() + try: + proc = subprocess.Popen( + [apm_command, *args], + cwd=str(cwd), + stdin=slave_fd, + stdout=slave_fd, + stderr=slave_fd, + close_fds=True, + ) + finally: + os.close(slave_fd) + + os.write(master_fd, stdin_text.encode("utf-8")) + + deadline = time.monotonic() + timeout + chunks: list[bytes] = [] + try: + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + proc.kill() + raise TimeoutError(f"apm {' '.join(args)} did not exit within {timeout}s") + ready, _, _ = select.select([master_fd], [], [], min(0.5, remaining)) + if ready: + try: + chunk = os.read(master_fd, 4096) + except OSError: + chunk = b"" + if not chunk: + break + chunks.append(chunk) + if proc.poll() is not None: + # Drain anything still buffered before exiting the loop. + while True: + ready, _, _ = select.select([master_fd], [], [], 0.1) + if not ready: + break + try: + chunk = os.read(master_fd, 4096) + except OSError: + chunk = b"" + if not chunk: + break + chunks.append(chunk) + break + finally: + os.close(master_fd) + + proc.wait(timeout=5) + return proc.returncode, b"".join(chunks).decode("utf-8", errors="replace") diff --git a/tests/unit/commands/test_install_context.py b/tests/unit/commands/test_install_context.py index ef76543ef..002a2dd02 100644 --- a/tests/unit/commands/test_install_context.py +++ b/tests/unit/commands/test_install_context.py @@ -67,6 +67,9 @@ class TestInstallContextFields: "only_packages", "manifest_snapshot", "snapshot_manifest_path", + # issue #1203: --frozen + plan-callback for `apm update` + "frozen", + "plan_callback", ) def test_all_required_fields_present(self): diff --git a/tests/unit/commands/test_update_command.py b/tests/unit/commands/test_update_command.py new file mode 100644 index 000000000..7cd40f567 --- /dev/null +++ b/tests/unit/commands/test_update_command.py @@ -0,0 +1,184 @@ +"""Unit tests for the ``apm update`` Click command. + +Issue: https://github.com/microsoft/apm/issues/1203 (P0). + +These tests mock the underlying ``_install_apm_dependencies`` so the +focus is on: + +* Plan callback wiring (assume_yes / dry-run / non-TTY paths). +* Back-compat shim: ``apm update`` outside an apm.yml project forwards + to ``apm self-update``. +* Mutex enforcement on ``apm install --frozen --update``. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.install.plan import PlanEntry, UpdatePlan + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +def _stub_plan_with_changes() -> UpdatePlan: + return UpdatePlan( + entries=( + PlanEntry( + dep_key="o/r", + action="update", + display_name="o/r", + old_resolved_ref="main", + new_resolved_ref="main", + old_resolved_commit="a" * 40, + new_resolved_commit="b" * 40, + ), + ) + ) + + +def _make_apm_yml(project_dir: Path) -> None: + (project_dir / "apm.yml").write_text( + "name: test\nversion: 1.0.0\ndependencies:\n apm:\n - microsoft/apm\n" + ) + + +# ----------------------------------------------------------------------------- +# apm update -- core flow +# ----------------------------------------------------------------------------- + + +class TestUpdateDryRun: + def test_dry_run_renders_plan_without_install(self, runner, tmp_path): + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + captured = {} + + def fake_install(_apm, **kwargs): + cb = kwargs["plan_callback"] + proceed = cb(_stub_plan_with_changes()) + captured["proceeded"] = proceed + from apm_cli.models.results import InstallResult + + return InstallResult() + + with patch( + "apm_cli.commands.install._install_apm_dependencies", side_effect=fake_install + ): + result = runner.invoke(cli, ["update", "--dry-run"]) + + assert result.exit_code == 0, result.output + assert "Update plan" in result.output + assert "Dry run" in result.output + assert captured["proceeded"] is False + + +class TestUpdateAssumeYes: + def test_yes_skips_prompt_and_proceeds(self, runner, tmp_path): + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + captured = {} + + def fake_install(_apm, **kwargs): + cb = kwargs["plan_callback"] + captured["proceeded"] = cb(_stub_plan_with_changes()) + from apm_cli.models.results import InstallResult + + return InstallResult(installed_count=1) + + with patch( + "apm_cli.commands.install._install_apm_dependencies", side_effect=fake_install + ): + result = runner.invoke(cli, ["update", "--yes"]) + + assert result.exit_code == 0, result.output + assert captured["proceeded"] is True + + +class TestUpdateNonTty: + def test_non_tty_aborts_without_yes_flag(self, runner, tmp_path): + """No --yes + non-TTY stdin -> exit 1 (CI-safe failure, do not mutate). + + Regression guard for the exit-code bug: non-TTY callers must see + a non-zero exit code so CI pipelines fail fast on accidental + 'apm update' invocations. + """ + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + + def fake_install(_apm, **kwargs): + cb = kwargs["plan_callback"] + # The callback should sys.exit(1) -- propagate as SystemExit + cb(_stub_plan_with_changes()) + from apm_cli.models.results import InstallResult + + return InstallResult() + + with patch( + "apm_cli.commands.install._install_apm_dependencies", side_effect=fake_install + ): + result = runner.invoke(cli, ["update"]) + + assert result.exit_code == 1, result.output + assert "non-interactive" in result.output + + +class TestUpdateNoChanges: + def test_unchanged_plan_short_circuits(self, runner, tmp_path): + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + + def fake_install(_apm, **kwargs): + cb = kwargs["plan_callback"] + proceed = cb(UpdatePlan(entries=())) + assert proceed is False + from apm_cli.models.results import InstallResult + + return InstallResult() + + with patch( + "apm_cli.commands.install._install_apm_dependencies", side_effect=fake_install + ): + result = runner.invoke(cli, ["update"]) + + assert result.exit_code == 0, result.output + assert "already at their latest" in result.output + + +# ----------------------------------------------------------------------------- +# apm update outside an apm.yml project -> back-compat shim +# ----------------------------------------------------------------------------- + + +class TestUpdateBackCompatShim: + def test_update_without_apm_yml_forwards_to_self_update(self, runner, tmp_path): + with runner.isolated_filesystem(temp_dir=tmp_path): + with patch("apm_cli.commands.self_update.self_update.callback") as mock_self_update: + mock_self_update.return_value = None + result = runner.invoke(cli, ["update"]) + + assert "self-update" in result.output + assert mock_self_update.called + + +# ----------------------------------------------------------------------------- +# apm install --frozen / --update mutex +# ----------------------------------------------------------------------------- + + +class TestFrozenUpdateMutex: + def test_frozen_and_update_together_rejected(self, runner, tmp_path): + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + result = runner.invoke(cli, ["install", "--frozen", "--update"]) + + assert result.exit_code != 0 + assert "frozen" in result.output.lower() + assert "update" in result.output.lower() diff --git a/tests/unit/install/test_architecture_invariants.py b/tests/unit/install/test_architecture_invariants.py index 03a91540a..a4b0f3261 100644 --- a/tests/unit/install/test_architecture_invariants.py +++ b/tests/unit/install/test_architecture_invariants.py @@ -178,12 +178,19 @@ def test_install_py_under_legacy_budget(): re-exports are mechanical aliases over already-extracted helpers and add no new logic. The pending --mcp extraction will recover this budget. + + Issue #1203 (lockfile UX P0) raised 1950 -> 2010 to land the + ``--frozen`` flag, plan-callback plumbing, FrozenInstallError + exception handlers, and the no-op "Run 'apm update'" nudge required + by the new ``apm update`` command and CI-safe install flow. All + additions are entry-point glue at the Click handler boundary; the + actual logic lives in ``apm_cli/install/`` (plan, errors, service). """ install_py = Path(__file__).resolve().parents[3] / "src" / "apm_cli" / "commands" / "install.py" assert install_py.is_file() n = _line_count(install_py) - assert n <= 1950, ( - f"commands/install.py grew to {n} LOC (budget 1950). " + assert n <= 2010, ( + f"commands/install.py grew to {n} LOC (budget 2010). " "Do NOT trim cosmetically -- engage the python-architecture skill " "(.github/skills/python-architecture/SKILL.md) and propose an " "extraction into apm_cli/install/." diff --git a/tests/unit/install/test_frozen.py b/tests/unit/install/test_frozen.py new file mode 100644 index 000000000..011c80ad6 --- /dev/null +++ b/tests/unit/install/test_frozen.py @@ -0,0 +1,103 @@ +"""Unit tests for ``InstallService._enforce_frozen``. + +Issue: https://github.com/microsoft/apm/issues/1203 (P0). +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from apm_cli.deps.lockfile import LockedDependency, LockFile +from apm_cli.install.errors import FrozenInstallError +from apm_cli.install.request import InstallRequest +from apm_cli.install.service import InstallService +from apm_cli.models.dependency.reference import DependencyReference + + +def _write_lockfile(project_dir: Path, deps: list[LockedDependency]) -> None: + lock = LockFile( + lockfile_version="1", + generated_at="2025-01-01T00:00:00+00:00", + apm_version="0.0.0-test", + ) + for dep in deps: + lock.add_dependency(dep) + (project_dir / "apm.lock.yaml").write_text(lock.to_yaml()) + + +def _write_apm_yml(project_dir: Path) -> None: + (project_dir / "apm.yml").write_text("name: test\nversion: 1.0.0\n") + + +def _make_request(*, project_dir: Path, manifest_deps: list[DependencyReference]) -> InstallRequest: + pkg = MagicMock() + pkg.package_path = project_dir / "apm.yml" + pkg.get_apm_dependencies.return_value = manifest_deps + pkg.get_dev_apm_dependencies.return_value = [] + return InstallRequest(apm_package=pkg, frozen=True) + + +class TestEnforceFrozen: + def test_raises_when_lockfile_missing(self, tmp_path: Path): + _write_apm_yml(tmp_path) + req = _make_request(project_dir=tmp_path, manifest_deps=[]) + + with pytest.raises(FrozenInstallError, match=r"requires apm\.lock\.yaml"): + InstallService._enforce_frozen(req) + + def test_raises_when_manifest_dep_missing_from_lockfile(self, tmp_path: Path): + _write_apm_yml(tmp_path) + _write_lockfile(tmp_path, []) + dep = DependencyReference(repo_url="https://github.com/declared/r") + req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) + + with pytest.raises(FrozenInstallError, match="out of sync") as exc_info: + InstallService._enforce_frozen(req) + + assert any("declared/r" in r for r in exc_info.value.reasons) + + def test_succeeds_when_lockfile_has_all_manifest_deps(self, tmp_path: Path): + _write_apm_yml(tmp_path) + _write_lockfile( + tmp_path, + [ + LockedDependency( + repo_url="https://github.com/o/r", + resolved_ref="main", + resolved_commit="a" * 40, + depth=1, + ), + ], + ) + dep = DependencyReference(repo_url="https://github.com/o/r") + req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) + + InstallService._enforce_frozen(req) + + def test_orphan_lockfile_entries_dont_fail(self, tmp_path: Path): + """Mirrors npm ci: extra lock entries are tolerated; only direct deps must be present.""" + _write_apm_yml(tmp_path) + _write_lockfile( + tmp_path, + [ + LockedDependency( + repo_url="https://github.com/o/r", + resolved_ref="main", + resolved_commit="a" * 40, + depth=1, + ), + LockedDependency( + repo_url="https://github.com/orphan/r", + resolved_ref="main", + resolved_commit="b" * 40, + depth=1, + ), + ], + ) + dep = DependencyReference(repo_url="https://github.com/o/r") + req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) + + InstallService._enforce_frozen(req) diff --git a/tests/unit/install/test_no_policy_flag.py b/tests/unit/install/test_no_policy_flag.py index 4e9feb0d8..c70177ea5 100644 --- a/tests/unit/install/test_no_policy_flag.py +++ b/tests/unit/install/test_no_policy_flag.py @@ -328,8 +328,11 @@ def setup_method(self): def test_update_no_policy_flag_rejected(self): """`apm update --no-policy` exits non-zero with usage error. - ``apm update`` is the CLI self-updater (refreshes the apm binary), not a - dependency refresh. A `--no-policy` flag would be misleading and dead. + ``apm update`` is the dependency-graph refresh command (since + issue #1203). The CLI self-updater moved to ``apm self-update``. + ``apm update`` does not surface ``--no-policy`` because policy + enforcement is fixed-on for the refresh flow; users who need the + opt-out must use ``apm install --update --no-policy``. """ result = self.runner.invoke(cli, ["update", "--no-policy"]) assert result.exit_code != 0, f"Expected non-zero exit, got 0\nOutput: {result.output}" diff --git a/tests/unit/install/test_plan.py b/tests/unit/install/test_plan.py new file mode 100644 index 000000000..ac1413b0b --- /dev/null +++ b/tests/unit/install/test_plan.py @@ -0,0 +1,291 @@ +"""Unit tests for ``apm_cli.install.plan``. + +Covers ``build_update_plan``, ``render_plan_text``, and +``lockfile_satisfies_manifest``. The plan module is pure -- no I/O, +no network -- so all tests use in-memory fixtures. + +Issue: https://github.com/microsoft/apm/issues/1203 +""" + +from __future__ import annotations + +from apm_cli.deps.lockfile import LockedDependency, LockFile +from apm_cli.install.plan import ( + PlanEntry, + UpdatePlan, + build_update_plan, + lockfile_satisfies_manifest, + render_plan_text, +) +from apm_cli.models.dependency.reference import DependencyReference +from apm_cli.models.dependency.types import GitReferenceType, ResolvedReference + + +def _new_lockfile() -> LockFile: + return LockFile( + lockfile_version="1", + generated_at="2025-01-01T00:00:00+00:00", + apm_version="0.0.0-test", + ) + + +def _locked( + repo_url: str, ref: str, commit: str, files: list[str] | None = None +) -> LockedDependency: + return LockedDependency( + repo_url=repo_url, + resolved_ref=ref, + resolved_commit=commit, + depth=1, + deployed_files=files or [], + ) + + +def _resolved_dep(repo_url: str, ref: str, commit: str | None) -> DependencyReference: + """Construct a DependencyReference with resolved_reference populated.""" + dep = DependencyReference(repo_url=repo_url, reference=ref) + dep.resolved_reference = ResolvedReference( + original_ref=ref, + ref_type=GitReferenceType.BRANCH, + ref_name=ref, + resolved_commit=commit, + ) + return dep + + +# ----------------------------------------------------------------------------- +# build_update_plan +# ----------------------------------------------------------------------------- + + +class TestBuildUpdatePlan: + def test_unchanged_dep_when_ref_and_commit_match(self): + lock = _new_lockfile() + lock.add_dependency(_locked("https://github.com/o/r", "main", "a" * 40)) + deps = [_resolved_dep("https://github.com/o/r", "main", "a" * 40)] + + plan = build_update_plan(lock, deps) + + assert plan.has_changes is False + assert len(plan.entries) == 1 + assert plan.entries[0].action == "unchanged" + + def test_update_when_commit_advances(self): + lock = _new_lockfile() + lock.add_dependency( + _locked("https://github.com/o/r", "main", "a" * 40, [".github/skills/x/SKILL.md"]) + ) + deps = [_resolved_dep("https://github.com/o/r", "main", "b" * 40)] + + plan = build_update_plan(lock, deps) + + assert plan.has_changes is True + entry = plan.entries[0] + assert entry.action == "update" + assert entry.old_resolved_commit == "a" * 40 + assert entry.new_resolved_commit == "b" * 40 + assert entry.deployed_files == (".github/skills/x/SKILL.md",) + + def test_add_when_dep_not_in_lockfile(self): + lock = _new_lockfile() + deps = [_resolved_dep("https://github.com/new/r", "main", "c" * 40)] + + plan = build_update_plan(lock, deps) + + assert plan.has_changes is True + assert plan.entries[0].action == "add" + assert plan.entries[0].new_resolved_commit == "c" * 40 + + def test_remove_when_locked_but_not_in_resolved(self): + lock = _new_lockfile() + lock.add_dependency(_locked("https://github.com/old/r", "main", "d" * 40)) + + plan = build_update_plan(lock, []) + + assert plan.has_changes is True + assert plan.entries[0].action == "remove" + assert plan.entries[0].old_resolved_commit == "d" * 40 + + def test_summary_counts_aggregate_correctly(self): + lock = _new_lockfile() + lock.add_dependency(_locked("https://github.com/u/r", "main", "a" * 40)) + lock.add_dependency(_locked("https://github.com/r/r", "main", "b" * 40)) + deps = [ + _resolved_dep("https://github.com/u/r", "main", "z" * 40), # update + _resolved_dep("https://github.com/n/r", "main", "n" * 40), # add + ] + + plan = build_update_plan(lock, deps) + + counts = plan.summary_counts + assert counts["update"] == 1 + assert counts["add"] == 1 + assert counts["remove"] == 1 + + def test_no_lockfile_returns_all_adds(self): + deps = [ + _resolved_dep("https://github.com/a/r", "main", "1" * 40), + _resolved_dep("https://github.com/b/r", "main", "2" * 40), + ] + + plan = build_update_plan(None, deps) + + assert all(e.action == "add" for e in plan.entries) + assert plan.has_changes is True + + def test_self_entry_ignored(self): + """The lockfile self-entry must not appear as a remove.""" + lock = _new_lockfile() + lock.local_deployed_files = [".github/instructions/x.md"] + + plan = build_update_plan(lock, []) + + assert plan.entries == () + + +# ----------------------------------------------------------------------------- +# render_plan_text +# ----------------------------------------------------------------------------- + + +class TestRenderPlanText: + def test_empty_plan_returns_empty_string(self): + assert render_plan_text(UpdatePlan(entries=())) == "" + + def test_unchanged_only_returns_empty_when_not_verbose(self): + plan = UpdatePlan( + entries=( + PlanEntry( + dep_key="o/r", + action="unchanged", + display_name="o/r", + old_resolved_ref="main", + new_resolved_ref="main", + old_resolved_commit="a" * 40, + new_resolved_commit="a" * 40, + ), + ) + ) + + assert render_plan_text(plan) == "" + + def test_update_entry_includes_ref_transition_and_files(self): + plan = UpdatePlan( + entries=( + PlanEntry( + dep_key="o/r", + action="update", + display_name="o/r", + old_resolved_ref="main", + new_resolved_ref="main", + old_resolved_commit="a" * 40, + new_resolved_commit="b" * 40, + deployed_files=(".github/skills/x/SKILL.md",), + ), + ) + ) + + text = render_plan_text(plan) + + assert "[~]" in text # update symbol + assert "o/r" in text + assert "aaaaaaa" in text # short old commit + assert "bbbbbbb" in text # short new commit + assert "SKILL.md" in text + assert "1 updated" in text # summary line + + def test_only_ascii_in_rendered_output(self): + """Encoding rule: printable ASCII only (Windows cp1252 safe).""" + plan = UpdatePlan( + entries=( + PlanEntry( + dep_key="o/r", + action="add", + display_name="o/r", + new_resolved_ref="main", + new_resolved_commit="c" * 40, + ), + ) + ) + + text = render_plan_text(plan) + + for ch in text: + assert ord(ch) <= 0x7E or ch in ("\n", "\r"), f"Non-ASCII char: {ch!r}" + + def test_verbose_includes_unchanged_count(self): + plan = UpdatePlan( + entries=( + PlanEntry( + dep_key="o/r", + action="unchanged", + display_name="o/r", + old_resolved_ref="main", + new_resolved_ref="main", + old_resolved_commit="a" * 40, + new_resolved_commit="a" * 40, + ), + PlanEntry( + dep_key="o/s", + action="update", + display_name="o/s", + old_resolved_commit="b" * 40, + new_resolved_commit="c" * 40, + ), + ) + ) + + text = render_plan_text(plan, verbose=True) + + assert "[=]" in text + assert "1 unchanged" in text + + +# ----------------------------------------------------------------------------- +# lockfile_satisfies_manifest +# ----------------------------------------------------------------------------- + + +class TestLockfileSatisfiesManifest: + def test_satisfied_when_all_manifest_deps_locked(self): + lock = _new_lockfile() + lock.add_dependency(_locked("https://github.com/o/r", "main", "a" * 40)) + manifest = [_resolved_dep("https://github.com/o/r", "main", None)] + + ok, reasons = lockfile_satisfies_manifest(lock, manifest) + + assert ok is True + assert reasons == [] + + def test_unsatisfied_when_manifest_dep_missing_from_lock(self): + lock = _new_lockfile() + manifest = [_resolved_dep("https://github.com/missing/r", "main", None)] + + ok, reasons = lockfile_satisfies_manifest(lock, manifest) + + assert ok is False + assert len(reasons) == 1 + assert "missing" in reasons[0] + + def test_local_deps_skipped(self): + """Local file deps have no remote ref, so they're skipped.""" + lock = _new_lockfile() + local = DependencyReference(repo_url="local", local_path="./vendor/pkg") + local.is_local = True + + ok, reasons = lockfile_satisfies_manifest(lock, [local]) + + assert ok is True + assert reasons == [] + + def test_orphan_lockfile_entries_do_not_fail_check(self): + """Lock has extra entries not in manifest -- structural check passes.""" + lock = _new_lockfile() + lock.add_dependency(_locked("https://github.com/o/r", "main", "a" * 40)) + lock.add_dependency(_locked("https://github.com/orphan/r", "main", "b" * 40)) + manifest = [_resolved_dep("https://github.com/o/r", "main", None)] + + ok, reasons = lockfile_satisfies_manifest(lock, manifest) + + assert ok is True + assert reasons == [] diff --git a/tests/unit/test_command_logger.py b/tests/unit/test_command_logger.py index 5799abcb1..ec8759800 100644 --- a/tests/unit/test_command_logger.py +++ b/tests/unit/test_command_logger.py @@ -256,6 +256,40 @@ def test_nothing_to_install_full(self, mock_success): logger.nothing_to_install() assert "up to date" in mock_success.call_args[0][0] + @patch("apm_cli.core.command_logger._rich_info") + @patch("apm_cli.core.command_logger._rich_success") + def test_nothing_to_install_nudges_when_lockfile_present(self, mock_success, mock_info): + """Nudge to 'apm update' fires when install is a no-op AND lockfile exists. + + Regression guard for the #1203 nudge branch: without this, users + would believe 'apm install' checks for newer refs. + """ + logger = InstallLogger(partial=False) + logger.nothing_to_install(lockfile_present=True, update_mode=False) + assert "up to date" in mock_success.call_args[0][0] + assert mock_info.called, "nudge line was not emitted" + nudge_msg = mock_info.call_args[0][0] + assert "apm update" in nudge_msg + assert "latest refs" in nudge_msg + + @patch("apm_cli.core.command_logger._rich_info") + @patch("apm_cli.core.command_logger._rich_success") + def test_nothing_to_install_no_nudge_in_update_mode(self, mock_success, mock_info): + """No nudge when the user already asked for an update.""" + logger = InstallLogger(partial=False) + logger.nothing_to_install(lockfile_present=True, update_mode=True) + assert "up to date" in mock_success.call_args[0][0] + assert not mock_info.called, "nudge should be suppressed in update mode" + + @patch("apm_cli.core.command_logger._rich_info") + @patch("apm_cli.core.command_logger._rich_success") + def test_nothing_to_install_no_nudge_without_lockfile(self, mock_success, mock_info): + """No nudge on first install (no lockfile yet).""" + logger = InstallLogger(partial=False) + logger.nothing_to_install(lockfile_present=False) + assert "up to date" in mock_success.call_args[0][0] + assert not mock_info.called + @patch("apm_cli.core.command_logger._rich_success") def test_install_summary_apm_only(self, mock_success): logger = InstallLogger() diff --git a/tests/unit/test_update_command.py b/tests/unit/test_update_command.py index af04edcf4..275dbf178 100644 --- a/tests/unit/test_update_command.py +++ b/tests/unit/test_update_command.py @@ -7,7 +7,7 @@ from click.testing import CliRunner -import apm_cli.commands.update as update_module +import apm_cli.commands.self_update as update_module from apm_cli.cli import cli @@ -39,9 +39,9 @@ def test_manual_update_command_uses_windows_installer(self): self.assertIn("aka.ms/apm-windows", command) self.assertIn("powershell", command.lower()) - @patch("apm_cli.commands.update.is_self_update_enabled", return_value=False) + @patch("apm_cli.commands.self_update.is_self_update_enabled", return_value=False) @patch( - "apm_cli.commands.update.get_self_update_disabled_message", + "apm_cli.commands.self_update.get_self_update_disabled_message", return_value="Update with: pixi update apm-cli", ) @patch("subprocess.run") @@ -54,7 +54,7 @@ def test_update_command_respects_disabled_policy( mock_enabled, ): """Disabled self-update policy should print guidance and skip installer.""" - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertIn("Update with: pixi update apm-cli", result.output) @@ -63,9 +63,9 @@ def test_update_command_respects_disabled_policy( @patch("requests.get") @patch("subprocess.run") - @patch("apm_cli.commands.update.get_version", return_value="0.6.3") - @patch("apm_cli.commands.update.shutil.which", return_value="powershell.exe") - @patch("apm_cli.commands.update.os.chmod") + @patch("apm_cli.commands.self_update.get_version", return_value="0.6.3") + @patch("apm_cli.commands.self_update.shutil.which", return_value="powershell.exe") + @patch("apm_cli.commands.self_update.os.chmod") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="0.7.0") def test_update_uses_powershell_installer_on_windows( self, @@ -84,7 +84,7 @@ def test_update_uses_powershell_installer_on_windows( mock_run.return_value = Mock(returncode=0) with patch.object(update_module.sys, "platform", "win32"): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertIn("Successfully updated to version 0.7.0", result.output) @@ -103,8 +103,8 @@ def test_update_uses_powershell_installer_on_windows( @patch("requests.get") @patch("subprocess.run") - @patch("apm_cli.commands.update.get_version", return_value="0.6.3") - @patch("apm_cli.commands.update.os.chmod") + @patch("apm_cli.commands.self_update.get_version", return_value="0.6.3") + @patch("apm_cli.commands.self_update.os.chmod") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="0.7.0") def test_update_uses_shell_installer_on_unix( self, @@ -123,9 +123,9 @@ def test_update_uses_shell_installer_on_unix( with ( patch.object(update_module.sys, "platform", "darwin"), - patch("apm_cli.commands.update.os.path.exists", return_value=True), + patch("apm_cli.commands.self_update.os.path.exists", return_value=True), ): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertIn("Successfully updated to version 0.7.0", result.output) @@ -240,46 +240,46 @@ def tearDown(self): os.environ["APM_TEMP_DIR"] = self._prev_apm_temp_dir self._tempdir.cleanup() - @patch("apm_cli.commands.update.get_version", return_value="unknown") + @patch("apm_cli.commands.self_update.get_version", return_value="unknown") def test_update_dev_version_warns_and_returns(self, mock_version): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertIn("development mode", result.output) - @patch("apm_cli.commands.update.get_version", return_value="unknown") + @patch("apm_cli.commands.self_update.get_version", return_value="unknown") def test_update_dev_version_check_flag_no_reinstall_hint(self, mock_version): """When --check is passed with dev version, reinstall hint should be suppressed.""" - result = self.runner.invoke(cli, ["update", "--check"]) + result = self.runner.invoke(cli, ["self-update", "--check"]) self.assertEqual(result.exit_code, 0) self.assertIn("development mode", result.output) self.assertNotIn("reinstall", result.output) @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value=None) - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") def test_update_cannot_fetch_latest_exits_1(self, mock_version, mock_latest): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 1) self.assertIn("Unable to fetch latest version", result.output) @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.0.0") - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") def test_update_already_on_latest(self, mock_version, mock_latest): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertIn("latest version", result.output) @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.1.0") - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") def test_update_check_flag_shows_available_no_install(self, mock_version, mock_latest): - result = self.runner.invoke(cli, ["update", "--check"]) + result = self.runner.invoke(cli, ["self-update", "--check"]) self.assertEqual(result.exit_code, 0) self.assertIn("1.0.0", result.output) self.assertIn("1.1.0", result.output) @patch("requests.get") @patch("subprocess.run") - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") - @patch("apm_cli.commands.update.os.chmod") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.os.chmod") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.1.0") def test_update_installer_failure_exits_1( self, mock_latest, mock_chmod, mock_version, mock_run, mock_get @@ -292,14 +292,14 @@ def test_update_installer_failure_exits_1( with ( patch.object(update_module.sys, "platform", "linux"), - patch("apm_cli.commands.update.os.path.exists", return_value=True), + patch("apm_cli.commands.self_update.os.path.exists", return_value=True), ): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 1) self.assertIn("Installation failed", result.output) - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.1.0") def test_update_requests_not_available_exits_1(self, mock_latest, mock_version): """When requests library is missing, exit with clear message.""" @@ -316,28 +316,28 @@ def mock_import(name, *args, **kwargs): patch("builtins.__import__", side_effect=mock_import), patch.object(update_module.sys, "platform", "linux"), ): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 1) self.assertIn("requests", result.output) @patch("requests.get") - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") - @patch("apm_cli.commands.update.os.chmod") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.os.chmod") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.1.0") def test_update_network_error_exits_1(self, mock_latest, mock_chmod, mock_version, mock_get): mock_get.side_effect = Exception("Network error") with patch.object(update_module.sys, "platform", "linux"): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 1) self.assertIn("Update failed", result.output) @patch("requests.get") @patch("subprocess.run") - @patch("apm_cli.commands.update.get_version", return_value="1.0.0") - @patch("apm_cli.commands.update.os.chmod") + @patch("apm_cli.commands.self_update.get_version", return_value="1.0.0") + @patch("apm_cli.commands.self_update.os.chmod") @patch("apm_cli.utils.version_checker.get_latest_version_from_github", return_value="1.1.0") def test_update_temp_file_cleanup_on_success( self, mock_latest, mock_chmod, mock_version, mock_run, mock_get @@ -358,10 +358,10 @@ def tracking_unlink(path): with ( patch.object(update_module.sys, "platform", "linux"), - patch("apm_cli.commands.update.os.path.exists", return_value=True), + patch("apm_cli.commands.self_update.os.path.exists", return_value=True), patch.object(update_module.os, "unlink", side_effect=tracking_unlink), ): - result = self.runner.invoke(cli, ["update"]) + result = self.runner.invoke(cli, ["self-update"]) self.assertEqual(result.exit_code, 0) self.assertEqual(len(deleted_paths), 1)