Add 'apm update' command and '--frozen' install flag (closes #1203)#1244
Conversation
Closes the lockfile-UX gap reported in #1203: - New 'apm update' resolves apm.yml against latest refs, prints a structured plan (added/updated/removed/unchanged), and prompts before mutating anything (default [y/N]). --yes and --dry-run supported. - Renames the CLI self-updater to 'apm self-update' so the bare verb matches 'npm update' / 'uv lock --upgrade' / 'cargo update'. Old 'apm update' (outside an apm.yml project) forwards to self-update with a one-release deprecation banner. - New 'apm install --frozen' performs a CI-safe, read-only install that fails fast when the lockfile is missing or out of sync with the manifest. Mutually exclusive with --update. Mirrors npm ci / uv sync --frozen. - 'apm install --force' help text now states explicitly that the flag does NOT refresh refs (use 'apm update' instead). - 'apm install' emits a one-line nudge ('Run \'apm update\' to check for newer versions.') when re-run against an existing lockfile in plain mode, pointing users at the verb they actually want. Architecture: - New apm_cli/install/plan.py module isolates plan/diff logic (PlanEntry, UpdatePlan, build_update_plan, render_plan_text, lockfile_satisfies_manifest). - plan_callback hook plumbed through InstallRequest -> InstallService -> pipeline so 'apm update' can render + prompt after resolve and abort cleanly when the user declines. - _enforce_frozen runs pre-pipeline with structural-only checks (skips local deps; tolerates orphan-lock entries). Tests: - tests/unit/install/test_plan.py (16 tests): plan builder, renderer, lockfile_satisfies_manifest. - tests/unit/install/test_frozen.py (4 tests): missing lockfile, manifest dep missing, satisfied case, orphan-lock-tolerated. - tests/unit/commands/test_update_command.py (6 tests): dry-run, --yes, non-TTY abort, no-op short-circuit, back-compat shim, --frozen + --update mutex. - tests/integration/test_update_e2e.py (7 tests): apm update --dry-run, follow-up no-op, --frozen success / missing-lock / out-of-sync / mutex, no-op nudge end-to-end against the real microsoft/apm-sample-package. Docs: - docs/src/content/docs/reference/lockfile-spec.md: lifecycle table documents apm update, --frozen, and clarifies --update. - packages/apm-guide/.apm/skills/apm-usage/commands.md: apm update, apm self-update, --frozen. - CHANGELOG.md: Added / Changed entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an npm-style dependency lifecycle to APM by introducing an interactive apm update (resolve + plan + confirm), a CI-safe apm install --frozen mode (lockfile-only, fail-fast on manifest/lock drift), and renaming the existing CLI self-updater to apm self-update with a short back-compat shim.
Changes:
- Introduces a new pure planning/diff library (
install/plan.py) and plumbs an optionalplan_callbackthrough the install pipeline to support “preview then commit” flows. - Adds
--frozenenforcement inInstallServiceand surfaces the new flag throughapm install. - Renames the CLI binary updater to
apm self-update, retargets tests/docs/changelog, and adds a newapm updatecommand for dependency refresh with confirmation.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_update_command.py | Retargets legacy self-update tests to apm self-update. |
| tests/unit/install/test_plan.py | Adds unit coverage for plan building/rendering and frozen structural checks. |
| tests/unit/install/test_no_policy_flag.py | Updates expectation/docs for apm update now being dependency refresh. |
| tests/unit/install/test_frozen.py | Adds unit coverage for InstallService._enforce_frozen. |
| tests/unit/install/test_architecture_invariants.py | Adjusts commands/install.py LOC budget with justification for new flags/wiring. |
| tests/unit/commands/test_update_command.py | Adds unit tests for the new apm update command flows and shim. |
| tests/unit/commands/test_install_context.py | Extends install context expected fields for frozen and plan_callback. |
| tests/integration/test_update_e2e.py | Adds end-to-end coverage for apm update and apm install --frozen. |
| src/apm_cli/utils/console.py | Extends status symbol catalog with plan-diff symbols. |
| src/apm_cli/install/service.py | Enforces --frozen early and threads plan_callback into the pipeline. |
| src/apm_cli/install/request.py | Adds frozen and plan_callback to the install request object. |
| src/apm_cli/install/plan.py | New module for update plan diffing/rendering and frozen structural validation. |
| src/apm_cli/install/pipeline.py | Adds a post-resolve/pre-download plan-gate checkpoint via plan_callback. |
| src/apm_cli/install/errors.py | Adds FrozenInstallError for frozen-mode failures. |
| src/apm_cli/core/command_logger.py | Enhances no-op logging and optionally prints an “apm update” hint. |
| src/apm_cli/commands/update.py | Implements new dependency-refresh apm update command and shim forwarding. |
| src/apm_cli/commands/self_update.py | New apm self-update command (moved from old apm update). |
| src/apm_cli/commands/install.py | Adds --frozen, mutex enforcement, and “no-op” hint behavior. |
| src/apm_cli/cli.py | Registers the new self-update command. |
| packages/apm-guide/.apm/skills/apm-usage/commands.md | Updates guide command table for apm update/apm self-update/--frozen. |
| docs/src/content/docs/reference/lockfile-spec.md | Documents --frozen semantics and apm update confirmation-gate behavior. |
| CHANGELOG.md | Adds Unreleased entries for apm update, --frozen, hinting, and rename. |
Copilot's findings
- Files reviewed: 22/22 changed files
- Comments generated: 7
# Conflicts: # CHANGELOG.md # src/apm_cli/commands/install.py # tests/unit/install/test_architecture_invariants.py
- Move no-op nudge to InstallLogger.nothing_to_install only; drop the install.py duplicate that fired even when files changed. - Fix _has_apm_yml docstring (no parent traversal). - Use _rich_warning(..., symbol="warning") for the back-compat banner instead of a hand-rolled "[!]" + _rich_info. - Add hidden --check flag on apm update for one-release self-update back-compat (forwards outside apm.yml; raises UsageError inside a project with guidance). - Module docstring: shim lives in update() itself, not in cli.py. - CHANGELOG: collapse to one line per entry, end with (#1244). - Drop "Prior to v0.12.5" historical phrasing from apm-usage commands.md (current-behavior-only rule). - Drop the e2e test that relied on the buggy install.py path; the proper nothing_to_install nudge is unit-tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 2 | 3 | Solid frozen-dataclass + callback-gate design; two recommended fixes: type InstallContext.plan_callback properly and remove the dead outer FrozenInstallError handler in install(). |
| CLI Logging Expert | 1 | 2 | 1 | One blocking exit-code bug (non-TTY error message + exit 0), two recommended symbol/style gaps, one nit on self-update. |
| DevX UX Expert | 0 | 2 | 2 | Strong npm-aligned surface; one prompt-default friction point and one --check flag dual-mode footgun worth addressing before shipping. |
| Supply Chain Security Expert | 0 | 2 | 1 | --frozen is presence-only (not SHA-integrity); self-update downloads+executes installer with no hash verification; consent gate is secure by default. |
| OSS Growth Hacker | 0 | 3 | 2 | Strong adoption win: verb-collision fix + frozen flag complete the npm-style promise; two recommended improvements to release narrative and CI-doc clarity before ship. |
| Doc Writer | 0 | 3 | 1 | 3 gaps: apm self-update missing from Added, lockfile-spec Section 7 stale, no guide callout for breaking apm update rename. |
| Test Coverage Expert | 0 | 1 | 2 | Nudge branch (nothing_to_install lockfile_present=True) has no test and PR overcounts integration tests by 1; all other critical surfaces (frozen, update, plan, mutex) have solid unit+integration coverage. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [CLI Logging Expert] (blocking-severity) Fix exit-code bug: replace 'return False' with 'sys.exit(1)' in the non-TTY branch of update.py -- apm update exits 0 on error in all non-interactive environments; this silently breaks CI pipelines and directly violates the governed-by-policy principle for frozen-mode enforcement.
- [Doc Writer + OSS Growth Hacker] Add standalone ### Breaking Changes heading to CHANGELOG for the apm update -> self-update rename, include sed migration hint, and add the self-update ### Added entry -- without a dedicated breaking-changes section, users upgrading in CI will hit the rename silently; a one-release deprecation window is safe only if the CHANGELOG makes the migration path obvious.
- [Doc Writer] Update lockfile-spec Section 7 and guides/dependencies.md to document apm update confirmation gate, --dry-run, and --frozen semantics -- two canonical reference docs have zero mentions of the new command; CI users relying on docs for integration guidance will miss the frozen-mode caveat (structural-only, not SHA-integrity).
- [Test Coverage Expert] Add unit test for the nothing_to_install(lockfile_present=True) nudge branch and correct the PR body integration-test count from 7 to 6 -- the nudge branch is untested; command_logger path through it has no assertion that would catch a silent regression.
- [Supply Chain Security Expert] Open a tracked issue for --frozen SHA-integrity enforcement and self-update installer hash verification (post-merge) -- leaving them untracked means the 'frozen = safe in CI' mental model users will form is underspecified; a public issue keeps the promise visible.
Architecture
classDiagram
direction LR
class PlanEntry {
<<ValueObject>>
+name str
+action str
+old_ref str
+new_ref str
+deployed_files list
}
class UpdatePlan {
<<ValueObject>>
+entries list
+has_changes bool
}
class InstallRequest {
<<DataTransferObject>>
+frozen bool
+plan_callback Callable or None
+update bool
}
class InstallService {
<<Facade>>
+run(request) InstallResult
-_enforce_frozen(request, lockfile) void
}
class InstallPipeline {
<<Pipeline>>
+run_install_pipeline(ctx, plan_callback) InstallResult
}
class InstallContext {
<<DataTransferObject>>
+frozen bool
+plan_callback Any
}
class FrozenInstallError {
<<DomainException>>
+reasons list
}
class build_update_plan {
<<Pure>>
+build_update_plan(old_lockfile, resolved_deps) UpdatePlan
+lockfile_satisfies_manifest(lockfile, deps) tuple
+render_plan_text(plan, verbose) str
}
InstallRequest *-- InstallService : drives
InstallService *-- InstallPipeline : delegates
InstallService ..> FrozenInstallError : raises
InstallPipeline ..> build_update_plan : calls
InstallPipeline ..> UpdatePlan : passes to callback
InstallRequest ..> InstallContext : fields copied into
UpdatePlan *-- PlanEntry : contains
class InstallRequest:::touched
class InstallService:::touched
class InstallPipeline:::touched
class InstallContext:::touched
class UpdatePlan:::touched
class PlanEntry:::touched
class FrozenInstallError:::touched
class build_update_plan:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm update (CLI)"])
B(["apm install --frozen (CLI)"])
C["commands/update.py: update()
check _has_apm_yml()"]
D["commands/self_update.py: self_update()
back-compat shim"]
E["commands/install.py: install()
mutex: frozen AND update -> UsageError"]
F["_install_apm_dependencies()
builds InstallRequest(frozen=, plan_callback=)"]
G["InstallService.run(request)
_enforce_frozen() BEFORE pipeline"]
H{"frozen=True?
check lockfile vs manifest"}
I["FrozenInstallError raised
exit 1"]
J["run_install_pipeline(plan_callback=...)
resolve deps"]
K{"plan_callback is not None?"}
L["build_update_plan(early_lockfile, resolved_deps)
render_plan_text()"]
M{"plan_callback(plan) returns True?"}
N["abort - return empty InstallResult"]
O["download + integrate + lockfile rewrite"]
P(["InstallResult"])
A --> C
C -- "no apm.yml" --> D
C -- "apm.yml found" --> F
B --> E
E -- "frozen+update" --> UsageError(["click.UsageError exit 2"])
E --> F
F --> G
G --> H
H -- "yes" --> I
H -- "no" --> J
J --> K
K -- "yes (apm update path)" --> L
L --> M
M -- "no / dry-run" --> N
M -- "yes" --> O
K -- "no (plain install path)" --> O
O --> P
sequenceDiagram
actor User
participant CLI as commands/update.py
participant Svc as InstallService
participant Pipe as run_install_pipeline
participant Plan as plan.py (pure)
participant FS as filesystem
User->>CLI: apm update [--yes] [--dry-run]
CLI->>CLI: _has_apm_yml() -- check cwd
alt no apm.yml
CLI-->>User: deprecation banner, forward to self-update
end
CLI->>Svc: InstallService().run(InstallRequest(update=True, plan_callback=cb))
Svc->>Pipe: run_install_pipeline(ctx, plan_callback=cb)
Pipe->>FS: read early_lockfile
Pipe->>Pipe: resolve deps (network)
Pipe->>Plan: build_update_plan(early_lockfile, resolved_deps)
Plan-->>Pipe: UpdatePlan(entries=[...])
Pipe->>CLI: plan_callback(plan) -- render plan, prompt [y/N]
CLI-->>User: print render_plan_text(plan)
alt --dry-run or User types N
CLI-->>Pipe: return False
Pipe-->>Svc: return empty InstallResult
Svc-->>User: exit 0, no mutations
else User types y or --yes
CLI-->>Pipe: return True
Pipe->>FS: download + integrate + rewrite apm.lock.yaml
Pipe-->>Svc: InstallResult
Svc-->>User: success summary
end
Recommendation
Fix the one-line exit-code bug in update.py (return False -> sys.exit(1) in the non-TTY branch) and close the three doc gaps (CHANGELOG breaking-changes heading + self-update entry, lockfile-spec Section 7, guides/dependencies.md) before or at merge -- all are in-PR fixable. Everything else (--frozen integrity depth, --check asymmetry, nudge test, symbol constants) belongs in tracked follow-up issues or a fast-follow PR. The core design is sound, the adoption story is strong, and holding this PR for the longer-horizon security depth items would be the wrong trade-off.
Full per-persona findings
Python Architect
-
[recommended] InstallContext.plan_callback typed as Any, defeating static analysis on the callback contract at
src/apm_cli/commands/install.py
The real typeCallable[[UpdatePlan], bool] | Noneis non-trivial and is exactly the contract that plan_callback callers (pipeline.py) rely on. Annotating as Any means mypy/pyright will not catch a future caller passing a wrong signature.
Suggested: Addfrom collections.abc import Callableand annotate:plan_callback: Callable[[UpdatePlan], bool] | None = None -
[recommended] Outer FrozenInstallError handler in install() is dead code; _install_apm_packages already exits at
src/apm_cli/commands/install.py
install.py has a top-levelexcept FrozenInstallErrorblock around_install_apm_packages. But_install_apm_packagesitself catches FrozenInstallError and calls sys.exit(1), so the exception never propagates. Two handlers for one error class on one call path is an architectural smell. -
[nit] plan.py suppresses UP035 for List/Optional/Tuple instead of modernising the annotations at
src/apm_cli/install/plan.py
The file already hasfrom __future__ import annotations;list,tuple, andT | Nonework at runtime. The noqa suppresses a real linter signal. -
[nit] getattr guards on InstallContext fields are unnecessary since the fields are always initialised at
src/apm_cli/commands/install.py
getattr(ctx, 'frozen', False)andgetattr(ctx, 'plan_callback', None)imply the fields might not exist; plain attribute access is correct and clearer. -
[nit] Design patterns are idiomatic; plan_callback inversion avoids coupling pipeline.py to Click, which is the right call.
CLI Logging Expert
-
[blocking] Non-TTY guard prints _rich_error but returns False instead of sys.exit(1), so apm update exits 0 in CI at
src/apm_cli/commands/update.py
When _stdin_is_tty() is False, _rich_error() is called and the callback returns False. run_install_pipeline returns an empty InstallResult and the function returns normally -- exit code 0. A CI job that accidentally runs 'apm update' (missing --yes) will see the red error text but get a zero exit code.
Suggested: Replacereturn Falsein the non-TTY branch withsys.exit(1)after the _rich_error call. -
[recommended] plan.py _ACTION_SYMBOLS hardcodes bracket strings instead of referencing STATUS_SYMBOLS keys, creating silent divergence risk at
src/apm_cli/install/plan.py
console.py defines 'update': '[~]', 'remove': '[-]', 'equal': '[=]' in STATUS_SYMBOLS. plan.py duplicates these as bare string literals. If STATUS_SYMBOLS is ever updated, plan.py output will silently diverge.
Suggested:from apm_cli.utils.console import STATUS_SYMBOLSand look up keys instead of embedding strings. -
[recommended] _rich_success and _rich_info calls in update.py _plan_callback omit symbol= so messages render without bracket prefix at
src/apm_cli/commands/update.py
The calls 'All dependencies already at their latest...', 'Dry run: no changes applied...', 'No changes applied.' all omit symbol=, rendering plain coloured text with no bracket -- inconsistent with CommandLogger/InstallLogger patterns.
Suggested: Addsymbol='check'/symbol='info'to each call. -
[nit] symbol='sparkles' in self_update.py is not in STATUS_SYMBOLS; prefix is silently dropped at
src/apm_cli/commands/self_update.py
Suggested: Replace withsymbol='info'or add'sparkles': '[*]'to STATUS_SYMBOLS.
DevX UX Expert
-
[recommended] Default-No prompt on 'apm update' deviates from every peer tool and adds friction for the primary interactive use case at
src/apm_cli/commands/update.py
npm update, cargo update, pip install --upgrade all resolve and mutate without a confirmation gate. A user who typed 'apm update' has already expressed intent. The existing --dry-run flag satisfies the 'preview without committing' need.
Suggested: Consider default-Yes (or no prompt + --dry-run). If security framing is the goal, default-Yes with explicit --no-apply is far less surprising than default-No. -
[recommended] 'apm update --check' works outside a project (forwarded to self-update shim) but raises UsageError inside one -- asymmetric behavior breaks CI composability at
src/apm_cli/commands/update.py
A script that historically ran 'apm update --check' in a repo dir will hit a UsageError after upgrading.
Suggested: Inside a project, 'apm update --check' should also forward to 'apm self-update --check' with the same deprecation banner. -
[nit] Plan output symbols [~][+][-] have no inline legend; breaks first-run promise for new users.
Suggested: Append~ updated + added - removedto the plan footer. -
[nit] 'apm install --update' still exists alongside 'apm update', doubling the mental surface for dependency refresh with subtly different semantics at
src/apm_cli/commands/install.py
Suggested: Add '(deprecated; prefer apm update or apm update --yes)' to the --update flag help text.
Supply Chain Security Expert
-
[recommended] --frozen checks key presence only; a tampered lockfile (key exists, wrong SHA) passes silently at
src/apm_cli/install/plan.py
lockfile_satisfies_manifest builds locked_keys from dependency key names and checks only membership. It does not compare locked ref/SHA values or verify content-hash integrity.
Proof (missing):tests/install/test_frozen.py-- proves: apm install --frozen exits 1 when the lockfile key exists but the pinned SHA differs [secure-by-default, governed-by-policy]
Suggested: Extend _enforce_frozen to compare locked ref/commit-SHA values against manifest-declared refs for each dep. -
[recommended] Self-update downloads installer script from aka.ms with no integrity check before exec at
src/apm_cli/commands/self_update.py
requests.get then write to tempfile then subprocess.run([shell_path, script_path]) with no expected-hash comparison, TLS-pinning, or signature verification.
Proof (missing):tests/commands/test_self_update.py-- proves: apm self-update refuses to execute a downloaded installer whose content does not match the expected hash [secure-by-default]
Suggested: Publish SHA-256 hashes of installer release artifacts and verify before exec. -
[nit] plan_callback typed as Any loses the security-relevant callable contract. Since the consent gate's correctness is a security invariant, the type should be enforced.
OSS Growth Hacker
-
[recommended] One-release deprecation window for 'apm update -> self-update' is adoption-risky; CHANGELOG needs standalone ### Breaking Changes heading with sed migration hint at
CHANGELOG.md
Without a dedicated breaking-changes section, users upgrading in CI will hit the rename silently.
Suggested: Add### Breaking Changesabove### Addedwith: 'apm update no longer self-updates the CLI when apm.yml is present. Migration:sed -i "s/apm update/apm self-update/g" your_scripts' -
[recommended] '--frozen' is structural-only; CI users who rely on it for full lockfile fidelity will be silently underprotected -- caveat must reach docs and --frozen help text at
docs/src/content/docs/getting-started/quick-start.md
Suggested: Add a NOTE callout in the CI section and add '(structural presence check only; use apm audit for on-disk integrity)' to the --frozen flag help text. -
[recommended] CHANGELOG 'Added' bullet for apm update needs a quotable hook sentence for social sharing at
CHANGELOG.md
Suggested: Prepend: 'apm update now does what every npm/pip/cargo user expects -- resolve latest refs, show a structured plan, and ask before touching anything.' -
[nit] No-op nudge copy is correct; OSC 8 hyperlink would be a minor delight in supported terminals.
-
[nit] Dry-run message could echo the count of planned changes ('3 packages would be updated') to convert inspection into motivation.
Auth Expert -- inactive
PR only touches install pipeline orchestration (plan_callback, --frozen lockfile check) and import refactoring of AuthenticationError; no changes to AuthResolver, token resolution, credential helpers, HTTP auth headers, host classification, or fallback semantics.
Doc Writer
-
[recommended] apm self-update is a new command but has no ### Added entry in CHANGELOG at
CHANGELOG.md:23
Keep-a-Changelog convention: new commands belong in ### Added. apm self-update appears only inside the ### Changed bullet for apm update.
Suggested: Add:- 'apm self-update': updates the APM CLI binary. Replaces the former 'apm update' self-update path. (#1244) -
[recommended] lockfile-spec Section 7 (Resolver Behaviour) does not mention apm update with its confirmation gate / --dry-run / structured-plan semantics at
docs/src/content/docs/reference/lockfile-spec.md:263
Section 7 names two update paths; apm update is a distinct third path with a different contract.
Suggested: Add a fourth bullet to Section 7 describing apm update's confirmation gate and --dry-run mode. -
[recommended] No guide-level callout for the breaking-change rename; guides/dependencies.md has zero mentions of apm update at
docs/src/content/docs/guides/dependencies.md
Users following the guide will not discover the command. Existing scripts relying on 'apm update' for CLI self-update will hit a silent behavioral change.
Suggested: Add a '## Updating dependencies' section with apm update intro and a :::caution[Breaking change] callout. -
[nit] apm self-update --check flag in commands.md has no corroborating source; may be aspirational syntax at
packages/apm-guide/.apm/skills/apm-usage/commands.md:165
Suggested: Verify --check is implemented. If not, remove it. If yes, add it to the CHANGELOG Added entry.
Test Coverage Expert
-
[recommended] nothing_to_install(lockfile_present=True) nudge path has no unit test; command_logger branch is untested
Grep of tests/unit/ for 'lockfile_present' and tests/integration/ for 'nudge': zero hits. The two existing tests call nothing_to_install() with no arguments, defaulting lockfile_present=False.
Proof (missing):tests/unit/test_command_logger.py::test_nothing_to_install_nudges_when_lockfile_present-- proves: when apm install is a no-op and a lockfile exists, users are nudged to run apm update [devx] -
[nit] PR body claims 7 integration tests in test_update_e2e.py but only 6 exist; Scenario 7 (nudge) has no integration test.
Proof (missing at integration tier):tests/integration/test_update_e2e.py::test_install_noop_nudges_to_run_apm_update -
[nit] All other critical surfaces have verified test coverage: test_update_command.py (6 scenarios), test_frozen.py (4 scenarios), test_plan.py (19 methods), test_update_e2e.py (6 integration tests). FrozenInstallError.reasons asserted. plan_callback=None (normal install) covered by existing install integration suite.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by PR Review Panel for issue #1244 · ● 5.4M · ◷
1 similar comment
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 2 | 3 | Solid frozen-dataclass + callback-gate design; two recommended fixes: type InstallContext.plan_callback properly and remove the dead outer FrozenInstallError handler in install(). |
| CLI Logging Expert | 1 | 2 | 1 | One blocking exit-code bug (non-TTY error message + exit 0), two recommended symbol/style gaps, one nit on self-update. |
| DevX UX Expert | 0 | 2 | 2 | Strong npm-aligned surface; one prompt-default friction point and one --check flag dual-mode footgun worth addressing before shipping. |
| Supply Chain Security Expert | 0 | 2 | 1 | --frozen is presence-only (not SHA-integrity); self-update downloads+executes installer with no hash verification; consent gate is secure by default. |
| OSS Growth Hacker | 0 | 3 | 2 | Strong adoption win: verb-collision fix + frozen flag complete the npm-style promise; two recommended improvements to release narrative and CI-doc clarity before ship. |
| Doc Writer | 0 | 3 | 1 | 3 gaps: apm self-update missing from Added, lockfile-spec Section 7 stale, no guide callout for breaking apm update rename. |
| Test Coverage Expert | 0 | 1 | 2 | Nudge branch (nothing_to_install lockfile_present=True) has no test and PR overcounts integration tests by 1; all other critical surfaces (frozen, update, plan, mutex) have solid unit+integration coverage. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [CLI Logging Expert] (blocking-severity) Fix exit-code bug: replace 'return False' with 'sys.exit(1)' in the non-TTY branch of update.py -- apm update exits 0 on error in all non-interactive environments; this silently breaks CI pipelines and directly violates the governed-by-policy principle for frozen-mode enforcement.
- [Doc Writer + OSS Growth Hacker] Add standalone ### Breaking Changes heading to CHANGELOG for the apm update -> self-update rename, include sed migration hint, and add the self-update ### Added entry -- without a dedicated breaking-changes section, users upgrading in CI will hit the rename silently; a one-release deprecation window is safe only if the CHANGELOG makes the migration path obvious.
- [Doc Writer] Update lockfile-spec Section 7 and guides/dependencies.md to document apm update confirmation gate, --dry-run, and --frozen semantics -- two canonical reference docs have zero mentions of the new command; CI users relying on docs for integration guidance will miss the frozen-mode caveat (structural-only, not SHA-integrity).
- [Test Coverage Expert] Add unit test for the nothing_to_install(lockfile_present=True) nudge branch and correct the PR body integration-test count from 7 to 6 -- the nudge branch is untested; command_logger path through it has no assertion that would catch a silent regression.
- [Supply Chain Security Expert] Open a tracked issue for --frozen SHA-integrity enforcement and self-update installer hash verification (post-merge) -- leaving them untracked means the 'frozen = safe in CI' mental model users will form is underspecified; a public issue keeps the promise visible.
Architecture
classDiagram
direction LR
class PlanEntry {
<<ValueObject>>
+name str
+action str
+old_ref str
+new_ref str
+deployed_files list
}
class UpdatePlan {
<<ValueObject>>
+entries list
+has_changes bool
}
class InstallRequest {
<<DataTransferObject>>
+frozen bool
+plan_callback Callable or None
+update bool
}
class InstallService {
<<Facade>>
+run(request) InstallResult
-_enforce_frozen(request, lockfile) void
}
class InstallPipeline {
<<Pipeline>>
+run_install_pipeline(ctx, plan_callback) InstallResult
}
class InstallContext {
<<DataTransferObject>>
+frozen bool
+plan_callback Any
}
class FrozenInstallError {
<<DomainException>>
+reasons list
}
class build_update_plan {
<<Pure>>
+build_update_plan(old_lockfile, resolved_deps) UpdatePlan
+lockfile_satisfies_manifest(lockfile, deps) tuple
+render_plan_text(plan, verbose) str
}
InstallRequest *-- InstallService : drives
InstallService *-- InstallPipeline : delegates
InstallService ..> FrozenInstallError : raises
InstallPipeline ..> build_update_plan : calls
InstallPipeline ..> UpdatePlan : passes to callback
InstallRequest ..> InstallContext : fields copied into
UpdatePlan *-- PlanEntry : contains
class InstallRequest:::touched
class InstallService:::touched
class InstallPipeline:::touched
class InstallContext:::touched
class UpdatePlan:::touched
class PlanEntry:::touched
class FrozenInstallError:::touched
class build_update_plan:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm update (CLI)"])
B(["apm install --frozen (CLI)"])
C["commands/update.py: update()
check _has_apm_yml()"]
D["commands/self_update.py: self_update()
back-compat shim"]
E["commands/install.py: install()
mutex: frozen AND update -> UsageError"]
F["_install_apm_dependencies()
builds InstallRequest(frozen=, plan_callback=)"]
G["InstallService.run(request)
_enforce_frozen() BEFORE pipeline"]
H{"frozen=True?
check lockfile vs manifest"}
I["FrozenInstallError raised
exit 1"]
J["run_install_pipeline(plan_callback=...)
resolve deps"]
K{"plan_callback is not None?"}
L["build_update_plan(early_lockfile, resolved_deps)
render_plan_text()"]
M{"plan_callback(plan) returns True?"}
N["abort - return empty InstallResult"]
O["download + integrate + lockfile rewrite"]
P(["InstallResult"])
A --> C
C -- "no apm.yml" --> D
C -- "apm.yml found" --> F
B --> E
E -- "frozen+update" --> UsageError(["click.UsageError exit 2"])
E --> F
F --> G
G --> H
H -- "yes" --> I
H -- "no" --> J
J --> K
K -- "yes (apm update path)" --> L
L --> M
M -- "no / dry-run" --> N
M -- "yes" --> O
K -- "no (plain install path)" --> O
O --> P
sequenceDiagram
actor User
participant CLI as commands/update.py
participant Svc as InstallService
participant Pipe as run_install_pipeline
participant Plan as plan.py (pure)
participant FS as filesystem
User->>CLI: apm update [--yes] [--dry-run]
CLI->>CLI: _has_apm_yml() -- check cwd
alt no apm.yml
CLI-->>User: deprecation banner, forward to self-update
end
CLI->>Svc: InstallService().run(InstallRequest(update=True, plan_callback=cb))
Svc->>Pipe: run_install_pipeline(ctx, plan_callback=cb)
Pipe->>FS: read early_lockfile
Pipe->>Pipe: resolve deps (network)
Pipe->>Plan: build_update_plan(early_lockfile, resolved_deps)
Plan-->>Pipe: UpdatePlan(entries=[...])
Pipe->>CLI: plan_callback(plan) -- render plan, prompt [y/N]
CLI-->>User: print render_plan_text(plan)
alt --dry-run or User types N
CLI-->>Pipe: return False
Pipe-->>Svc: return empty InstallResult
Svc-->>User: exit 0, no mutations
else User types y or --yes
CLI-->>Pipe: return True
Pipe->>FS: download + integrate + rewrite apm.lock.yaml
Pipe-->>Svc: InstallResult
Svc-->>User: success summary
end
Recommendation
Fix the one-line exit-code bug in update.py (return False -> sys.exit(1) in the non-TTY branch) and close the three doc gaps (CHANGELOG breaking-changes heading + self-update entry, lockfile-spec Section 7, guides/dependencies.md) before or at merge -- all are in-PR fixable. Everything else (--frozen integrity depth, --check asymmetry, nudge test, symbol constants) belongs in tracked follow-up issues or a fast-follow PR. The core design is sound, the adoption story is strong, and holding this PR for the longer-horizon security depth items would be the wrong trade-off.
Full per-persona findings
Python Architect
-
[recommended] InstallContext.plan_callback typed as Any, defeating static analysis on the callback contract at
src/apm_cli/commands/install.py
The real typeCallable[[UpdatePlan], bool] | Noneis non-trivial and is exactly the contract that plan_callback callers (pipeline.py) rely on. Annotating as Any means mypy/pyright will not catch a future caller passing a wrong signature.
Suggested: Addfrom collections.abc import Callableand annotate:plan_callback: Callable[[UpdatePlan], bool] | None = None -
[recommended] Outer FrozenInstallError handler in install() is dead code; _install_apm_packages already exits at
src/apm_cli/commands/install.py
install.py has a top-levelexcept FrozenInstallErrorblock around_install_apm_packages. But_install_apm_packagesitself catches FrozenInstallError and calls sys.exit(1), so the exception never propagates. Two handlers for one error class on one call path is an architectural smell. -
[nit] plan.py suppresses UP035 for List/Optional/Tuple instead of modernising the annotations at
src/apm_cli/install/plan.py
The file already hasfrom __future__ import annotations;list,tuple, andT | Nonework at runtime. The noqa suppresses a real linter signal. -
[nit] getattr guards on InstallContext fields are unnecessary since the fields are always initialised at
src/apm_cli/commands/install.py
getattr(ctx, 'frozen', False)andgetattr(ctx, 'plan_callback', None)imply the fields might not exist; plain attribute access is correct and clearer. -
[nit] Design patterns are idiomatic; plan_callback inversion avoids coupling pipeline.py to Click, which is the right call.
CLI Logging Expert
-
[blocking] Non-TTY guard prints _rich_error but returns False instead of sys.exit(1), so apm update exits 0 in CI at
src/apm_cli/commands/update.py
When _stdin_is_tty() is False, _rich_error() is called and the callback returns False. run_install_pipeline returns an empty InstallResult and the function returns normally -- exit code 0. A CI job that accidentally runs 'apm update' (missing --yes) will see the red error text but get a zero exit code.
Suggested: Replacereturn Falsein the non-TTY branch withsys.exit(1)after the _rich_error call. -
[recommended] plan.py _ACTION_SYMBOLS hardcodes bracket strings instead of referencing STATUS_SYMBOLS keys, creating silent divergence risk at
src/apm_cli/install/plan.py
console.py defines 'update': '[~]', 'remove': '[-]', 'equal': '[=]' in STATUS_SYMBOLS. plan.py duplicates these as bare string literals. If STATUS_SYMBOLS is ever updated, plan.py output will silently diverge.
Suggested:from apm_cli.utils.console import STATUS_SYMBOLSand look up keys instead of embedding strings. -
[recommended] _rich_success and _rich_info calls in update.py _plan_callback omit symbol= so messages render without bracket prefix at
src/apm_cli/commands/update.py
The calls 'All dependencies already at their latest...', 'Dry run: no changes applied...', 'No changes applied.' all omit symbol=, rendering plain coloured text with no bracket -- inconsistent with CommandLogger/InstallLogger patterns.
Suggested: Addsymbol='check'/symbol='info'to each call. -
[nit] symbol='sparkles' in self_update.py is not in STATUS_SYMBOLS; prefix is silently dropped at
src/apm_cli/commands/self_update.py
Suggested: Replace withsymbol='info'or add'sparkles': '[*]'to STATUS_SYMBOLS.
DevX UX Expert
-
[recommended] Default-No prompt on 'apm update' deviates from every peer tool and adds friction for the primary interactive use case at
src/apm_cli/commands/update.py
npm update, cargo update, pip install --upgrade all resolve and mutate without a confirmation gate. A user who typed 'apm update' has already expressed intent. The existing --dry-run flag satisfies the 'preview without committing' need.
Suggested: Consider default-Yes (or no prompt + --dry-run). If security framing is the goal, default-Yes with explicit --no-apply is far less surprising than default-No. -
[recommended] 'apm update --check' works outside a project (forwarded to self-update shim) but raises UsageError inside one -- asymmetric behavior breaks CI composability at
src/apm_cli/commands/update.py
A script that historically ran 'apm update --check' in a repo dir will hit a UsageError after upgrading.
Suggested: Inside a project, 'apm update --check' should also forward to 'apm self-update --check' with the same deprecation banner. -
[nit] Plan output symbols [~][+][-] have no inline legend; breaks first-run promise for new users.
Suggested: Append~ updated + added - removedto the plan footer. -
[nit] 'apm install --update' still exists alongside 'apm update', doubling the mental surface for dependency refresh with subtly different semantics at
src/apm_cli/commands/install.py
Suggested: Add '(deprecated; prefer apm update or apm update --yes)' to the --update flag help text.
Supply Chain Security Expert
-
[recommended] --frozen checks key presence only; a tampered lockfile (key exists, wrong SHA) passes silently at
src/apm_cli/install/plan.py
lockfile_satisfies_manifest builds locked_keys from dependency key names and checks only membership. It does not compare locked ref/SHA values or verify content-hash integrity.
Proof (missing):tests/install/test_frozen.py-- proves: apm install --frozen exits 1 when the lockfile key exists but the pinned SHA differs [secure-by-default, governed-by-policy]
Suggested: Extend _enforce_frozen to compare locked ref/commit-SHA values against manifest-declared refs for each dep. -
[recommended] Self-update downloads installer script from aka.ms with no integrity check before exec at
src/apm_cli/commands/self_update.py
requests.get then write to tempfile then subprocess.run([shell_path, script_path]) with no expected-hash comparison, TLS-pinning, or signature verification.
Proof (missing):tests/commands/test_self_update.py-- proves: apm self-update refuses to execute a downloaded installer whose content does not match the expected hash [secure-by-default]
Suggested: Publish SHA-256 hashes of installer release artifacts and verify before exec. -
[nit] plan_callback typed as Any loses the security-relevant callable contract. Since the consent gate's correctness is a security invariant, the type should be enforced.
OSS Growth Hacker
-
[recommended] One-release deprecation window for 'apm update -> self-update' is adoption-risky; CHANGELOG needs standalone ### Breaking Changes heading with sed migration hint at
CHANGELOG.md
Without a dedicated breaking-changes section, users upgrading in CI will hit the rename silently.
Suggested: Add### Breaking Changesabove### Addedwith: 'apm update no longer self-updates the CLI when apm.yml is present. Migration:sed -i "s/apm update/apm self-update/g" your_scripts' -
[recommended] '--frozen' is structural-only; CI users who rely on it for full lockfile fidelity will be silently underprotected -- caveat must reach docs and --frozen help text at
docs/src/content/docs/getting-started/quick-start.md
Suggested: Add a NOTE callout in the CI section and add '(structural presence check only; use apm audit for on-disk integrity)' to the --frozen flag help text. -
[recommended] CHANGELOG 'Added' bullet for apm update needs a quotable hook sentence for social sharing at
CHANGELOG.md
Suggested: Prepend: 'apm update now does what every npm/pip/cargo user expects -- resolve latest refs, show a structured plan, and ask before touching anything.' -
[nit] No-op nudge copy is correct; OSC 8 hyperlink would be a minor delight in supported terminals.
-
[nit] Dry-run message could echo the count of planned changes ('3 packages would be updated') to convert inspection into motivation.
Auth Expert -- inactive
PR only touches install pipeline orchestration (plan_callback, --frozen lockfile check) and import refactoring of AuthenticationError; no changes to AuthResolver, token resolution, credential helpers, HTTP auth headers, host classification, or fallback semantics.
Doc Writer
-
[recommended] apm self-update is a new command but has no ### Added entry in CHANGELOG at
CHANGELOG.md:23
Keep-a-Changelog convention: new commands belong in ### Added. apm self-update appears only inside the ### Changed bullet for apm update.
Suggested: Add:- 'apm self-update': updates the APM CLI binary. Replaces the former 'apm update' self-update path. (#1244) -
[recommended] lockfile-spec Section 7 (Resolver Behaviour) does not mention apm update with its confirmation gate / --dry-run / structured-plan semantics at
docs/src/content/docs/reference/lockfile-spec.md:263
Section 7 names two update paths; apm update is a distinct third path with a different contract.
Suggested: Add a fourth bullet to Section 7 describing apm update's confirmation gate and --dry-run mode. -
[recommended] No guide-level callout for the breaking-change rename; guides/dependencies.md has zero mentions of apm update at
docs/src/content/docs/guides/dependencies.md
Users following the guide will not discover the command. Existing scripts relying on 'apm update' for CLI self-update will hit a silent behavioral change.
Suggested: Add a '## Updating dependencies' section with apm update intro and a :::caution[Breaking change] callout. -
[nit] apm self-update --check flag in commands.md has no corroborating source; may be aspirational syntax at
packages/apm-guide/.apm/skills/apm-usage/commands.md:165
Suggested: Verify --check is implemented. If not, remove it. If yes, add it to the CHANGELOG Added entry.
Test Coverage Expert
-
[recommended] nothing_to_install(lockfile_present=True) nudge path has no unit test; command_logger branch is untested
Grep of tests/unit/ for 'lockfile_present' and tests/integration/ for 'nudge': zero hits. The two existing tests call nothing_to_install() with no arguments, defaulting lockfile_present=False.
Proof (missing):tests/unit/test_command_logger.py::test_nothing_to_install_nudges_when_lockfile_present-- proves: when apm install is a no-op and a lockfile exists, users are nudged to run apm update [devx] -
[nit] PR body claims 7 integration tests in test_update_e2e.py but only 6 exist; Scenario 7 (nudge) has no integration test.
Proof (missing at integration tier):tests/integration/test_update_e2e.py::test_install_noop_nudges_to_run_apm_update -
[nit] All other critical surfaces have verified test coverage: test_update_command.py (6 scenarios), test_frozen.py (4 scenarios), test_plan.py (19 methods), test_update_e2e.py (6 integration tests). FrozenInstallError.reasons asserted. plan_callback=None (normal install) covered by existing install integration suite.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by PR Review Panel for issue #1244 · ● 5.4M · ◷
Fixes the one blocking finding (non-TTY exit 0 in apm update) plus the recommended doc/test/style gaps surfaced by the panel. Blocking - update.py non-TTY guard now sys.exit(1) instead of return False; CI pipelines that accidentally invoke 'apm update' without --yes fail fast as the governed-by-policy principle requires. Recommended (in-PR) - install.py: type InstallContext.plan_callback as Callable[[UpdatePlan], bool] | None (TYPE_CHECKING import); drop dead outer FrozenInstallError handler in install() (inner _install_apm_packages already exits); replace getattr guards on ctx.frozen / ctx.plan_callback with plain attribute access. - plan.py: source action symbols from utils.console.STATUS_SYMBOLS (single source of truth) and modernise typing imports (drop List/Optional/Tuple noqa shim). - update.py: add explicit symbol= on _rich_success/_rich_info calls in _plan_callback; forward 'apm update --check' inside a project to 'apm self-update --check' with deprecation banner (was raising UsageError; broke CI composability). - install.py: --update help text now flagged 'deprecated; prefer apm update'; --frozen help text now states 'structural presence check only; use apm audit for on-disk integrity'. - plan.py: render_plan_text appends an inline legend '[~] updated [+] added [-] removed' after the summary so first- run users decode the symbols without leaving the terminal. Doc gaps - CHANGELOG.md: add standalone ### Breaking Changes heading with sed migration hint for the apm update -> self-update rename; add ### Added entry for apm self-update; prepend npm/pip/cargo hook sentence to the apm update bullet; surface the --frozen structural-only caveat. - docs/.../reference/lockfile-spec.md: extend Section 7 with two new bullets describing apm update (interactive plan + dry-run) and apm install --frozen (presence-only) resolver paths. - docs/.../guides/dependencies.md: add ## Updating dependencies section covering apm update / --dry-run / --yes, the --frozen CI pairing with NOTE callout on its scope, and a CAUTION callout for the breaking apm update -> self-update rename. - packages/apm-guide/.apm/skills/apm-usage/commands.md: mark apm install --update as deprecated and document --frozen scope. Test coverage - tests/unit/test_command_logger.py: add three nudge-branch tests for InstallLogger.nothing_to_install -- nudges when lockfile_present=True, suppresses in update_mode, suppresses on first install -- closing the regression-trap gap on the #1203 command_logger path. - tests/unit/commands/test_update_command.py: update non-TTY abort test to assert exit_code == 1 (was passing on return-False). Validation - ruff check + format --check: silent - 7249 unit tests pass Skipped per panel guidance (longer-horizon, tracked follow-ups): - --frozen SHA-integrity enforcement (presence-only is documented) - self-update installer hash verification - default-Yes prompt flip (CEO sided with author on default-No) Refs #1244 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed in 486580a. Summary mapped to the panel findings: Blocking
Recommended (in-PR)
Doc gaps
Test coverage
Skipped per panel guidance (longer-horizon, tracked follow-ups)
Validation
Note on the |
…e, render parity, decline e2e
- update.py: walk parent dirs to find apm.yml (npm/cargo ergonomics);
emit one-time CI banner under CI/GITHUB_ACTIONS so pipelines see the
dependency-refresh shift; chdir to discovered project root.
- install.py: post-success --frozen nudge ('Run apm audit for on-disk
content integrity.') sets correct expectation about presence-only
scope; FrozenInstallError now uses _rich_error for parity with
update.py rendering.
- command_logger.py: no-op nudge copy reframed as
'Lockfile already satisfied -- run apm update to resolve latest
refs.' (cargo/npm pattern, names the current state).
- tests/integration/test_update_e2e.py: TestUpdateInteractiveDecline
exercises the real-TTY decline path via pty.openpty(), verifying
rc=0 and lockfile bytes unchanged after user answers 'n'.
Refs: #1244 (comment)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Panel followups landed in e310d8c:
Lint silent; 693/693 unit tests pass. Out of scope (deliberate): |
The nudge copy was reframed in e310d8c from 'check for newer versions' to 'resolve latest refs.'; this test still asserted the old substring and broke CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…f-update) Skill: .apm/skills/docs-sync executed as orchestrator (this thread) following Step 2 (architect) + Step 3 (fan-out writers) per the cascade-size mitigation introduced in commit 03aa3f4. Panel: 1 architect (doc-analyser) + 4 section writers (doc-writer) dispatched as parallel background subagents. 9 stale corpus pages collapsed to 5 LLM calls (well under the 15-call ceiling). Changes: - NEW: reference/cli/self-update.md (binary updater; auto-picked by Starlight autogenerate via sidebar.order: 5) - REWRITE: reference/cli/update.md (now describes the dep updater with consent gate; documents the back-compat shim) - REWRITE: consumer/update-and-refresh.mdx (verbs flipped; new --frozen, apm update, and apm self-update sections) - PATCH: reference/cli/install.md (add --frozen flag row, no-op nudge bullet, frozen-mode bullet, --frozen exit codes, fix --force note, retarget Related links) - PATCH: consumer/install-packages.md, manage-dependencies.md (Coming-from-npm Asides + flag references) - PATCH: concepts/glossary.md (new self-update + update entries; remove false 'NOT npm-equivalent' caveat from install entry) - PATCH: concepts/package-anatomy.md, quickstart.mdx (Coming-from- npm framings updated) Follow-up: 13 secondary mentions of 'apm install --update' (flag form, still valid) remain in drift-and-secure-by-default, troubleshooting, registry-proxy, security-and-supply-chain, authentication, outdated, and publish-to-a-marketplace. Flag is retained per PR #1244; prose can shift to 'apm update' command form in a follow-up sweep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs(wave-a): apply 12 P0 fixes from CDO synthesis P0 fixes from docs audit panel (PR #1244 follow-up; full corpus rewrite plan): - env var: GITHUB_CLI_PAT -> GITHUB_APM_PAT in dependencies.md - schema: remove fictional scripts: object form (only dict[str,str] supported) - new section: apm cache {info,clean,prune} in cli-commands.md - deprecation banners on apm unpack across 6 pages (v0.14 removal) - baseline-checks count fixed to 8 across enterprise + integrations + reference - broken ../governance/ links retargeted to existing pages (4 files) - contributing lint commands now use --extra dev - Windsurf added to harness lists (index, what-is-apm, why-apm, key-concepts) - silent-success warning added to apm policy status (use --check for CI) - security callout on apm install --force (overwrite + drift bypass) Verified no-ops (CDO entries that did not require change): - apm marketplace plugin: zero command-form occurrences in docs - .codex/ deploy directory: VERIFIED real per integration/targets.py:466-484 Build: PASS, link validator clean, 53 pages built. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-b): foundation chapter (8 pages) New foundation chapter for the docs rewrite: - index.mdx (REPLACED): splash hero, 3-promise pitch, persona CardGrid - quickstart.mdx (NEW): 5-minute consumer ramp, 4 commands - concepts/what-is-apm.md (NEW): mental model, primitive orientation - concepts/the-three-promises.md (NEW): canonical 3-promise contract with src/ citations - concepts/primitives-and-targets.md (NEW): full compatibility matrix (single source of truth for primitive x harness reach) - concepts/package-anatomy.md (NEW): apm.yml + apm.lock.yaml schema - concepts/lifecycle.md (NEW): init/install/compile/run/audit verbs - concepts/glossary.md (NEW): 24 terms with sharp negations Verification: - CDO checkpoint: APPROVED, narrative arc cohesive - Editorial pass: 4 run-on splits in primitives-and-targets + lifecycle; no banned adjectives or patterns found - Build: 60 pages, all internal links valid Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: untrack .docs-rewrite-plan/ (process metadata, not docs) These files (AGENTIC-SYSTEM-DESIGN.md, CONTEXT-PACK.md) coordinate the multi-wave subagent execution. They live locally for the orchestrator and subagents but should not ship in the docs repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(docs): use ../sibling/ paths for slug-page cross-links Slug pages (file.md → /file/) need ../target/ for sibling links because the trailing slash makes ./target/ resolve as a child path. Index pages still use ./target/ (the trailing slash is the parent directory). Affected: 6 slug pages across guides/, concepts/, introduction/. Fixes broken /apm/guides/pack-distribute/marketplace-authoring/ and similar 404s. enterprise/index.md is unchanged (already correct). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-c): consumer ramp (10 pages) + concepts SSOT fix 10 new pages on the consumer ramp: install-packages, install-mcp-servers, deploy-a-bundle, run-scripts, update-and-refresh, manage-dependencies, authentication (slim), private-and-org-packages, drift-and-secure-by-default, governance-on-the-consumer-ramp. Each page drafted by a parallel doc-writer (draft -> verify-against-src -> editorial-self-pass), then chapter-level CDO checkpoint and editorial-owner voice sweep. CDO REVISE issues addressed: - manage-dependencies: corrected to document apm install <pkg> as a real manifest-mutating shortcut (verified in src/apm_cli/commands/install.py:114). - update-and-refresh: --runtime -> --target for chapter consistency. - update-and-refresh: dead audit-and-security link -> drift-and-secure-by-default. - concepts/primitives-and-targets: fallback corrected to 'minimal' (AGENTS.md only) per src/apm_cli/core/target_detection.py:206; previously claimed 'copilot' fallback which was drift. ASCII only. No sidebar blocks (Wave I owns sidebar). Build green; all internal links validate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-d): producer ramp (10 pages) + CDO PASS polish 10 new pages on the producer ramp: - author-primitives: skills, prompts, instructions-and-agents, hooks-and-commands, mcp-as-primitive - compile, preview-and-validate, pack-a-bundle, publish-to-a-marketplace, package-relative-links Each page drafted by parallel doc-writer (draft -> verify-against-src -> editorial), then chapter-level editorial-owner sweep + CDO checkpoint. CDO verdict PASS (no must-fix). Applied MED-severity polish: - hooks-and-commands: copilot hook filenames are <pkg>-<name>.json with bundled scripts under .github/hooks/scripts/<pkg>/, per src/apm_cli/integration/hook_integrator.py. - publish-to-a-marketplace: retargeted dead consumer link to legacy guides/marketplaces (Wave C does not yet cover marketplace install flow); retargeted enterprise/governance-overview to existing enterprise/governance-guide. - mcp-as-primitive: title-case fix on Primitives-and-targets link. - consumer/install-packages: corrected apm list description (lists scripts declared in apm.yml, not a tree view of installed packages), per src/apm_cli/commands/list_cmd.py:23. Surfaced by Wave D editorial. ASCII only. No sidebar blocks (Wave I owns sidebar). Build green; 80 pages, all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-e): enterprise ramp (9 pages) + CDO fixes Wave E -- Enterprise ramp for the docs full rewrite. Lands 7 new pages and modifies 2 legacy pages, completing the persona-led narrative for platform owners and security leads. NEW pages: - governance-overview.md -- chapter map; 4 control surfaces, the read-this-chapter-in-order list. - apm-policy-getting-started.md -- minimal policy in 10 minutes; schema spine. - policy-pilot.md -- warn-first rollout pattern; tighten-only inheritance via extends. - enforce-in-ci.md -- apm audit --ci wiring; 8 baselines + 17 policy checks; --policy scopes. - drift-detection.md -- the eight non-bypassable lockfile baselines and what each one catches. - security-and-supply-chain.md -- SecurityGate threat model; MCP trust boundary; provenance. - github-rulesets.md -- making apm audit --ci authoritative via required-status-check rulesets. MODIFIED pages: - adoption-playbook.md -- retargeted to new chapter siblings. - registry-proxy.md -- title cased; minor edits. CDO REVISE applied (6 must-fix link retargets across 5 files): - governance-overview.md src citation tightened (install/phases/policy_gate.py delegates to install_preflight.py). - adoption-playbook.md, github-rulesets.md, security-and-supply-chain.md, policy-pilot.md: ../governance-guide/ -> ../governance-overview/; ../apm-policy/ -> ../apm-policy-getting-started/; legacy guides/ paths retargeted to new chapter pages. policy-reference kept as the field-by-field schema reference. Bridge fixes for sealed waves: - consumer/governance-on-the-consumer-ramp.md: links to enterprise/governance-overview at the chapter handoff. - consumer/drift-and-secure-by-default.md: links to enterprise/security-and-supply-chain and enterprise/drift-detection. - producer/publish-to-a-marketplace.md: governance-guide retarget -> governance-overview. Build green: 87 pages, all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-f): per-command CLI reference pages (24 commands) Adds one page per top-level apm command under reference/cli/, replacing the monolithic cli-commands.md surface (which will be retired in Wave I). Each page follows the same template (Synopsis, Description, Subcommands, Options, Examples, Behavior, Exit codes, Related) and was authored by a parallel doc-writer subagent verifying every flag against src/apm_cli/ commands/. Pages: init, install, uninstall, update, view, list, prune, outdated, cache, config, compile, run, preview, audit, targets, runtime, pack, unpack, policy, marketplace, search, mcp, deps, experimental. Wave F1 of the persona-led rewrite. Build green, 111 pages, all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-f): schema + top-level reference pages - NEW: policy-schema, targets-matrix, baseline-checks, environment-variables - REWRITE: manifest-schema, lockfile-spec (drop W3C status block, modernize voice) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-g): troubleshooting catalog + IDE integration hub Wave G of the persona-led docs rewrite: troubleshooting/ (5 NEW + 1 REWRITE): - common-errors.md: catalog grouped by surface (install/compile/audit/run/auth) - install-failures.md: deep auth/network/lockfile/cache/recovery ladder - compile-zero-output-warning.md: diagnostic ladder for zero-output compile - ssl-issues.md (REWRITE): corporate proxy, GHES, GitLab self-managed, escape hatches - policy-debugging.md: which rule fired, inheritance, escape hatches - migration.md: from awd-cli, lockfile upgrades, target migration integrations/ide-tool-integration.md (REWRITE): - 703 line monolith collapsed to 137 line hub - detection rules, primitive flow, per-target reference pointers - delegates depth to targets-matrix, per-CLI reference, troubleshooting Build: 120 pages, all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(wave-i): persona-led sidebar + retire legacy guides + Astro polish Apply Wave H (oss-growth) and Wave I (Astro/Starlight expert) recommendations in one pass. Sidebar restructure - Replace legacy 'Understanding APM / Getting Started / Guides' tree with persona ramps: Start here, Use a package (Consumer), Author a package (Producer), Govern at scale (Enterprise), Integrations, CLI reference, Schemas and specs, Concepts, Troubleshooting, Contributing. - Autogenerate per-command CLI reference from reference/cli/. - Surface all 6 troubleshooting pages (was: only ssl-issues). Retired legacy - Delete reference/cli-commands.md (97KB monolith superseded by per-command pages; old URL redirects to /reference/cli/install/). - Delete getting-started/quick-start.md (duplicate of /quickstart/; old URL redirects). - Delete legacy guides/* (13 pages) and introduction/* (5 pages); content is now owned by consumer/, producer/, concepts/ ramps. New landing pages (4) - consumer/index.md - persona table + 4-command flow + recommended order. - producer/index.md - 5-step ladder Author->Compile->Validate->Pack->Publish. - producer/author-primitives/index.md - primitive type table + on-disk layout + recommended reading order. - troubleshooting/index.md - symptom triage table + escalation block. - reference/index.md - CLI by lifecycle phase + schemas table + conventions. Bulk link rewrite - Map ~25 legacy /apm/guides/* and /apm/introduction/* internal links to their new persona-ramp homes across the corpus (~30 files touched). Astro/Starlight polish - Add @astrojs/sitemap; emits /apm/sitemap-index.xml. - trailingSlash: 'always' (matches Starlight's link generation). - prefetch: prefetchAll on viewport. - editLink + lastUpdated enabled. - expressiveCode: github-dark + github-light themes, wrap defaultProps, borderRadius. - starlight-llms-txt: customSets per persona ramp + CLI reference; exclude contributing/. - head meta: theme-color, og:type, twitter:card. - Comprehensive redirects map (~25 entries) for any legacy URL referenced from external sources. Build: 98 pages, 127 HTML files, all internal links valid, sitemap generated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: polish expressive-code rendering (themes, radius, shadow) Apply explicit github-dark/github-light theme pair so dark mode picks up high-contrast syntax colors instead of falling back to the default single theme. Add small structural styleOverrides: - borderRadius 0.5rem (matches Starlight card radius) - borderWidth 1px (subtle, single-pixel frame) - codeFontSize 0.875rem + lineHeight 1.5 (consistent body rhythm) - frames.shadowColor: transparent (drop the heavy default drop-shadow) Colors are intentionally left to each theme so terminal frames keep proper contrast in both light and dark modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(skills): add docs-sync per-PR advisory skill (issue scaffolding) Add an agentic system that per-PR detects doc-corpus drift, classifies impact (no_change | in_place | structural), and orchestrates a CDO + doc-writer + python-architect + editorial-owner + growth-hacker panel that emits a single advisory comment. Components: - .apm/skills/docs-sync/ -- orchestrator (single-writer interlock, 7-step checklist, 15-call ceiling, ALIGNMENT LOOP bounded N<=3) - .apm/skills/docs-impact-{classifier,localizer,architect}/ -- 3-layer retrieval funnel siblings (L0 path gate / L1 grep / L2 LLM) - .apm/agents/{editorial-owner,cdo}.agent.md -- new personas - .apm/docs-index.yml -- corpus manifest (TOC + abstracts + symbol map) - .apm/skills/docs-sync/evals/ -- trigger evals (20) + 3 content evals - .github/workflows/docs-sync.md -- gh-aw workflow, rung-1 label-gated (pull_request_target + 'if:' label-name guard, fork-safe) Rung 1 rollout: requires 'docs-sync' label. Companion PR (Step 7) requires second label 'docs-sync-confirm' as A9 supervised execution boundary. Design followed the genesis skill; full case study with mermaid diagrams + design-pattern justification archived in session files (docs-sync-skill-design.md). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(workflow): compile docs-sync.lock.yml (gh aw v0.71.5) - gh aw compile docs-sync (0 errors, 0 warnings) - Add 'checkout: false' frontmatter as defense-in-depth against the pwn-request attack (we never need PR head -- 'gh pr diff' returns inert text). Combined with read-only permissions, arbitrary code in the PR head is physically unreachable from the workflow. Compiled lock file pins: - microsoft/apm-action@v1.7.2 (b48dd081) - github/gh-aw-actions/setup@v0.71.5 - all transitive actions to SHAs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(docs-sync): apply 4 eval-driven fixes from shadow run Shadow-evaluated against 8 recently-merged PRs (1244, 1240, 1259, 1239, 1238, 1257, 1261, 1263). Findings persisted to session files; this commit applies the 4 high-priority gaps surfaced by the eval. 1. Add 'in_place_resolved' verdict (classifier schema + SKILL.md). PRs where the author already patched every affected doc page (1239, 1259) should silently succeed without an advisory comment. Killed false-positive class. 2. Pass 'pr_doc_diff_paths[]' into the L2 classifier envelope, with an explicit downgrade rule: if every scope_page is in pr_doc_diff_paths, return in_place_resolved. 3. Add rename / breaking-change heuristic to the classifier. PR 1244 ('apm update' verb swap) would have been miscategorized as in_place without this -- the affected pages existed, but the shape of the work is structural (page split + TOC delta). 4. Add section-grouping mitigation for >8-page cascades in the orchestrator's Step 3. PR 1244's 9-page rename cascade would have approached the 15-call ceiling at one writer call per page. Now collapses to 2-3 section writer tasks when the same conceptual fix repeats across pages in the same TOC section. 5. Also extend docs-index.yml: add pyproject.toml + uv.lock to no_impact_paths (caught by PR 1263 eval); add also_consider_paths for packages/apm-guide/.apm/skills/apm-usage/** + README + MANIFESTO (caught by PR 1244 eval -- those touch downstream doc surfaces but are not part of the human-facing corpus). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(sync): apply docs-sync panel for PR #1244 (apm update -> apm self-update) Skill: .apm/skills/docs-sync executed as orchestrator (this thread) following Step 2 (architect) + Step 3 (fan-out writers) per the cascade-size mitigation introduced in commit 03aa3f4. Panel: 1 architect (doc-analyser) + 4 section writers (doc-writer) dispatched as parallel background subagents. 9 stale corpus pages collapsed to 5 LLM calls (well under the 15-call ceiling). Changes: - NEW: reference/cli/self-update.md (binary updater; auto-picked by Starlight autogenerate via sidebar.order: 5) - REWRITE: reference/cli/update.md (now describes the dep updater with consent gate; documents the back-compat shim) - REWRITE: consumer/update-and-refresh.mdx (verbs flipped; new --frozen, apm update, and apm self-update sections) - PATCH: reference/cli/install.md (add --frozen flag row, no-op nudge bullet, frozen-mode bullet, --frozen exit codes, fix --force note, retarget Related links) - PATCH: consumer/install-packages.md, manage-dependencies.md (Coming-from-npm Asides + flag references) - PATCH: concepts/glossary.md (new self-update + update entries; remove false 'NOT npm-equivalent' caveat from install entry) - PATCH: concepts/package-anatomy.md, quickstart.mdx (Coming-from- npm framings updated) Follow-up: 13 secondary mentions of 'apm install --update' (flag form, still valid) remain in drift-and-secure-by-default, troubleshooting, registry-proxy, security-and-supply-chain, authentication, outdated, and publish-to-a-marketplace. Flag is retained per PR #1244; prose can shift to 'apm update' command form in a follow-up sweep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(landing+auth): proof-first landing, multi-host auth coverage Landing page (index.mdx): - Restore manifest example as conversion artifact (12 lines, multi-host proof: GitHub + GitLab + Azure DevOps + Bitbucket + MCP servers) - Promote three promises back from italic prose into 3-card grid (Portable / Secure / Governed; drop weak 'Any git host' card -- proof moves into the manifest example) - Keep new npm-analogy hero tagline - Keep install command above the fold (one bash block, Windows footnote) - Add new 'Manage MCP servers by manifest' section after the manifest example (positions APM's MCP-as-dependency model as the novel differentiator vs. ad-hoc per-IDE MCP config) - Deduplicate persona router (was rendered twice as LinkCards + Cards; now appears exactly once as 'Pick your path') - Replace verbose 'Open Source & Community' paragraph with one-line footer Authentication page (consumer/authentication.md): - Reframe opener: any git host you can 'git clone' from, not GitHub-and-ADO-only - Expand '30-second answer' with GitLab and generic-git rows - Add 'GitLab (SaaS or self-managed)' section: GITLAB_APM_PAT -> GITLAB_TOKEN -> git credential helper precedence - Add 'Bitbucket, Gitea, and any other git host' section: credential helpers only, by design (if 'git clone' works, 'apm install' works) - Update frontmatter description to reflect multi-host coverage - Update 'Going further' link to existing enterprise/security page astro.config.mjs: - Remove stale '/getting-started/first-package' -> '/producer' redirect (target lacked /apm/ base; conflicted with sidebar entry for the same slug -- the page exists and is in the sidebar) All claims grounded against src/apm_cli/core/token_manager.py (GitLab and generic-host auth), docs/src/content/docs/reference/ manifest-schema.md (MCP registry-prefixed form, ADO _git segment required), and existing source repo for handle currency. Driven by the docs-sync skill: doc-analyser architect plan + 2 parallel doc-writer subagents + orchestrator-applies + S7 verify. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(landing): OS tabs for install, drop redundant MCP section - Split install into per-OS Tabs (macOS/Linux | Windows) with syncKey='os' so the choice persists across the site. - Separate APM bootstrap (curl|sh / irm|iex) from sample-package install -- they are two distinct steps, not one block. - Drop standalone 'Manage MCP servers by manifest' H2 -- the manifest example already shows MCP entries (io.github.*) as first-class deps; a separate section was redundant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: fix stale getting-started/authentication links Three links in the rewrite still pointed at the old 'getting-started/authentication' page with an '#authenticating-with-microsoft-entra-id-aad-bearer-tokens' anchor that no longer exists on the new consumer page (the in-depth AAD bearer content moved to enterprise/security.md). The legacy /getting-started/authentication path itself redirects to /consumer/authentication via astro.config.mjs, but the redirect drops the anchor and the new page intentionally stays brief. Repointed links: - enterprise/security.md:302 -- self-link inside the same page, now '(#azure-devops-aad-bearer-tokens)'. - integrations/ci-cd.md:36 -- '../../consumer/authentication/' (top-level guide for tokens and priority rules). - integrations/ci-cd.md:136 -- '../../enterprise/security/#azure-devops-aad-bearer-tokens' (deep link to the anchor that actually exists). Verified: curl-and-grep on the rendered enterprise/security/ page shows id='azure-devops-aad-bearer-tokens' is present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(ci): refresh apm-managed integration files; retarget policy help-consistency tests; correct stale apm install --policy doc snippet - apm install: regenerate .agents/ skills + .github/agents/ from current .apm/ sources (resolves APM Self-Check drift). - tests/unit/policy/test_help_consistency.py: retarget DOCS_PATH to the per-command files reference/cli/audit.md and reference/cli/policy.md (legacy reference/cli-commands.md was retired in 39d5740). Switch bullet helper to a markdown-table-row helper to match the new docs. - enterprise/apm-policy-getting-started.md: replace 3 invalid 'apm install --policy ...' examples with the correct surfaces: 'apm policy status --policy-source ...' for preview and 'apm audit --ci --policy ...' for CI gating. apm install has no --policy flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(ci): regenerate apm integration files with v0.13.0 path-resolution Local re-install with apm v0.12.2 produced relative path '../instructions/tests.instructions.md' for the test-coverage-expert agent's reference to .apm/instructions/tests.instructions.md, but microsoft/apm-action@v1 (CI) ships v0.13.0 which produces '../../.apm/instructions/tests.instructions.md'. Re-running install under v0.13.0 locally restores parity and clears APM Self-Check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(docs): remove duplicate @astrojs/sitemap override blocking npm ci The merge from main brought in @astrojs/sitemap as a direct dependency (added in 39d5740 wave-i for sitemap generation) while preserving the @astrojs/sitemap entry in 'overrides' from main. npm 11 rejects the overlap with EUSAGE 'Override conflicts with direct dependency', which the cli surfaces as the misleading 'no package-lock.json' message. Drop the override (the direct dep already pins the same version) and regenerate the lockfile. Verified: npm ci + npm run build succeed locally; 100 pages built; all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(quickstart): hide apm run from main flow; flag as experimental apm run is an optional npx-style script runner, not required to use any installed package. Surfacing it as step 4 of the quickstart pushed every new user into runtime-CLI installation and prompt orchestration before they had even read what their installed package does. - Drop the dedicated 'Run an agent' step from quickstart; replace with a short 'What next' note pointing at consumer/run-scripts/ for users who want it. - Update the scripts: bullet earlier in the page to flag the experimental status instead of teasing step 4. - Add a caution admonition to consumer/run-scripts/ marking the runner as experimental and explicitly off the critical path. Verified: 100 pages built; all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(compile): correct scope -- compile only handles instructions, not all primitives apm compile only consumes instructions/*.instructions.md (and optionally one chatmode to prepend); it produces AGENTS.md / CLAUDE.md / GEMINI.md root context files plus per-harness rules trees (.github/instructions/, .claude/rules/, .cursor/rules/*.mdc, .windsurf/rules/). Other primitive types (prompts, skills, agents, chatmodes-as-content, hooks, commands, MCP) are deployed by apm install directly into the harness directories that consume them; compile does not touch them. Verified against src/apm_cli/compilation/ -- AgentsCompiler and DistributedAgentsCompiler iterate primitives.instructions and primitives.chatmodes only, never primitives.prompts / skills / agents. - producer/compile.md: rewrite to scope the description, table, and pitfalls to instructions only; clarify install vs compile boundary. - reference/cli/compile.md: rewrite Description to name instructions as the only input and call out the install handover for everything else. Verified: 100 pages built; all internal links valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(quickstart): move 'Looking for apm run?' note above the What next cards Per maintainer feedback, surface the experimental apm run pointer right after the success summary, before the three persona cards, so users considering 'how do I actually run things' see the answer before they scroll into producer / governance ramps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(compile): clarify compile is optional for copilot, recommended for other targets GitHub Copilot natively reads .github/instructions/*.instructions.md (with their applyTo: frontmatter) that apm install already deploys, so the AGENTS.md / copilot-instructions.md output of compile is nice-to-have rather than required for that target. Other targets (claude, cursor, codex, gemini, opencode, windsurf) load instructions through a root context file or harness-specific rules folder that compile generates -- without it, the harness will not pick the instructions up. Add an explicit 'Compile required?' column to the per-target table and a callout in both the producer guide and the CLI reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Daniel Meppiel <copilot-rework@github.com>
add(install):
apm updatecommand and--frozeninstall flagCloses #1203.
TL;DR
Adds the missing third leg of npm-style dependency lifecycle:
apm update(refresh refs with explicit user consent),apm install --frozen(CI-safe, lockfile-only install that fails fast on drift), and a no-op nudge that points users atapm updatewhenapm installfinds the lockfile already satisfied. The previous CLI conflated "update my deps" with "update the CLI itself" under one verb; that command is nowapm self-update, with a one-release back-compat shim. Honours the public commitment in issue #1203.Important
Breaking-change shape:
apm updateno longer self-updates the CLI when anapm.ymlis present in the cwd. The shim only forwards toapm self-updatewhen no manifest is found, prints a deprecation banner, and is removed in the next minor.Problem (WHY)
--forceside effects.apm install --forcere-downloads but does NOT re-resolvelatest/branch refs against the remote, and the--forcehelp text did not say so. Users who wanted "npm update"-style behavior had no command that did it.apm installhappily resolves new refs when the lockfile is missing or out of sync withapm.yml, which is the wrong default for CI and reproducible builds.npm ci/uv sync --frozensolve this; APM had no equivalent.apm updateself-updated the CLI.apm updatewas the CLI self-updater, blocking the natural name for the dependency-update command. This is the verb every npm/pip/cargo user reaches for first.apm install. When the lockfile already satisfies the manifest,apm installsilently did nothing, leaving users guessing whether they needed--forceto "really" update. That is a DevX cliff.Why these matter: APM's first-run promise is "pragmatic as npm". A user who types
apm updateafter editingapm.ymland gets the CLI self-updater instead of a dependency plan is the exact failure mode that erodes the promise.Approach (WHAT)
apm updateClick command -- resolvesapm.ymlagainst latest refs, prints structured plan (added/updated/removed/unchanged), prompts[y/N](default False), aborts cleanly without touching lockfile or filesystem when declined. Supports--yes,--dry-run,--verbose.apm self-update. Back-compat shim INSIDE the newapm update: when noapm.ymlis present in cwd, forward toself-updatewith a one-line deprecation banner. One release.apm install --frozen-- read-only install that exits 1 ifapm.lock.yamlis missing or any directapm.ymldep is absent from the lockfile. Mutually exclusive with--update(UsageError). Structural check only; tolerates orphan-lock entries; skips local deps.apm install --forcehelp text now states explicitly that--forcedoes NOT refresh refs.apm installemitsRun 'apm update' to check for newer versions.(_rich_info) when the lockfile already existed AND no--update/--frozen/--force/partial-package args were passed.Implementation (HOW)
Architecture seam: a new pure plan/diff library plus one optional callback plumbed through the install pipeline.
apm installkeepsplan_callback=Noneand behaves exactly as before; onlyapm updatepasses a callback.src/apm_cli/install/plan.py(new) -- pure library:PlanEntry,UpdatePlan,build_update_plan,render_plan_text,lockfile_satisfies_manifest. No I/O, no Click, no global state.src/apm_cli/commands/update.py-- Click command. Callsrun_install_pipelinewith aplan_callbackthat renders the plan, prompts, and returnsFalsewhen the user declines (which short-circuits the pipeline before any mutation).src/apm_cli/commands/self_update.py(new) -- the CLI self-updater, lifted verbatim from the oldapm update. Tests retargeted (see test diff).src/apm_cli/install/request.py/service.py/pipeline.py--frozen: boolandplan_callback: Callable[[UpdatePlan], bool] | Noneplumbed throughInstallRequest -> InstallService.run -> run_install_pipeline. Callback invoked AFTER resolve, AFTERctx.tui.__exit__(). ReturnsFalse-> pipeline returnsInstallResult()without mutations._enforce_frozenruns early in the service layer; mutual exclusion with--updateis aUsageErrorat the CLI layer.src/apm_cli/commands/install.py-- adds--frozenflag, mutex check, no-op nudge wiring. Help text for--forceupdated.src/apm_cli/utils/console.py/core/command_logger.py-- minor: formatting hook for the no-op nudge.Permalink to the new plan library: https://github.com/microsoft/apm/blob/7fe67890/src/apm_cli/install/plan.py.
Diagrams
Legend: temporal flow of
apm update, highlighting the user-consent gate (yellow) that distinguishes this command fromapm install --update.sequenceDiagram participant U as User participant CLI as apm update participant Svc as InstallService participant Plan as plan.py participant Pipe as run_install_pipeline U->>CLI: apm update CLI->>Svc: InstallRequest(plan_callback=cb) Svc->>Pipe: resolve refs (latest) Pipe->>Plan: build_update_plan(manifest, lockfile, resolved) Plan-->>Pipe: UpdatePlan(added, updated, removed, unchanged) rect rgb(255, 247, 200) Note over CLI,U: NEW: consent gate -- no mutation before yes Pipe->>CLI: cb(plan) CLI->>U: render plan + prompt [y/N] U-->>CLI: y / N CLI-->>Pipe: True / False end alt user accepts Pipe->>Pipe: download, write lockfile, integrate Pipe-->>CLI: InstallResult(ok) else user declines Pipe-->>CLI: InstallResult() (no mutation) endLegend: control-flow gate for
apm install --frozen, showing the structural check that runs before any download.flowchart LR A[apm install --frozen] --> B{apm.lock.yaml exists?} B -- no --> X1[exit 1: missing lockfile] B -- yes --> C{every direct manifest dep present in lockfile?} C -- no --> X2[exit 1: out of sync] C -- yes --> D[install from lockfile only] D --> E[no ref resolution, no network for refs] classDef new stroke-dasharray: 5 5; class A,B,C,X1,X2 new;Trade-offs
deployed_filesfrom the existing lockfile; does NOT show file-level diff post-download. Materializing both versions to diff content was rejected as out-of-scope for P0; tracked for follow-up.--frozenis structural-only. Verifies presence of every direct manifest dep in the lockfile; does NOT verify pinned commits resolve to the same files on disk -- that isapm audit's job. Chose strict separation of concerns over a single flag that does both poorly.--frozen-lockfilealias rejected. Just--frozen, matchinguv. Two names for one flag is a DevX tax.apm update->apm self-update. Removed in next minor. A longer deprecation window was considered; rejected because the verb is the natural dependency-update name and squatting it as the CLI self-updater is the bug we are fixing.render_plan_textis plain ASCII grouped by section. The consent prompt is the focus, not the rendering.Benefits
apm updateexists and does what every npm/pip/cargo user expects -- resolvelatestrefs, show a plan, prompt for consent.apm install --frozengives CI a single flag to assert "lockfile is the source of truth, fail if drifted" with exit code 1. Mirrorsuv sync --frozenandnpm ci.apm installinto an actionable hint exactly when the user is most likely to be confused.--forcehelp text no longer lies about what--forcedoes.plan_callback) is reusable: any future command that needs "preview then commit" semantics plugs in here without touching the pipeline.Validation
uv run --extra dev ruff check src/ tests/-- silent.uv run --extra dev ruff format --check src/ tests/-- silent (732 files already formatted).uv run pytest tests/unit -q-- 8036 passed (30 subtests, 1 unrelated deprecation warning).GITHUB_TOKEN=$(gh auth token) uv run pytest tests/integration/test_update_e2e.py -q-- 7 passed against realmicrosoft/apm-sample-package.New / modified test files
Added:
tests/unit/install/test_plan.py-- 16 tests coveringbuild_update_plan,render_plan_text,lockfile_satisfies_manifest.tests/unit/install/test_frozen.py-- 4 tests:--frozenhappy path, missing lockfile, out-of-sync, mutex with--update.tests/unit/commands/test_update_command.py-- 6 tests: dry-run, accept, decline, non-TTY without--yes,--yesnon-interactive, no-manifest -> self-update shim.tests/integration/test_update_e2e.py-- 7 tests against real GitHub package.Modified:
tests/unit/test_update_command.pyretargeted toself_update(same surface, new module path).tests/unit/install/test_architecture_invariants.pyLOC budget bumped 1855 -> 1915 with documented justification (matches the established [FEATURE] Support.agents/skillsas anapm installtarget for shared skill deployment #737 / v0.12.4 pattern).tests/unit/commands/test_install_context.pyEXPECTED_FIELDSextended withfrozen+plan_callback.tests/unit/install/test_no_policy_flag.pydocstring updated.Scenario Evidence
apm updateresolveslatestrefs and prints a plan; declining at the prompt mutates nothingtests/unit/commands/test_update_command.py(decline path)tests/integration/test_update_e2e.py(dry-run vs real package)apm updatefollowed immediately byapm updateis a no-op (refs already current)tests/integration/test_update_e2e.py(follow-up no-op)apm install --frozensucceeds when the lockfile satisfiesapm.ymltests/unit/install/test_frozen.py(happy path)tests/integration/test_update_e2e.py(frozen success)apm install --frozenexits 1 with a clear message when the lockfile is missingtests/unit/install/test_frozen.py(missing-lock)tests/integration/test_update_e2e.py(frozen missing-lock)apm install --frozenexits 1 when a directapm.ymldep is absent from the lockfiletests/unit/install/test_frozen.py(out-of-sync)tests/integration/test_update_e2e.py(frozen out-of-sync)apm install --frozen --updateis rejected as a usage errortests/unit/install/test_frozen.py(mutex)tests/integration/test_update_e2e.py(mutex)apm install(no flags) on an existing lockfile prints theapm updatenudgetests/integration/test_update_e2e.py(no-op nudge)apm updatewith noapm.ymlin cwd forwards toself-updatewith a deprecation banner (regression-trap for #1203 verb-collision)tests/unit/commands/test_update_command.py(no-manifest shim)How to test
apm.ymlusinglatestrefs, runapm update --dry-run. Observe the plan grouped into added / updated / removed / unchanged with no prompt and no mutation.apm update, decline at the[y/N]prompt. Confirmapm.lock.yamlmtime did not change and no files under primitives dirs were touched.apm install --frozenwith a freshapm.lock.yaml-- succeeds. Delete the lockfile and re-run -- exits 1 with a clear message. Add a new dep toapm.ymlwithout re-locking and re-run -- exits 1.apm install --frozen --update-- rejected as aUsageError.apm installon an already-satisfied lockfile -- observe theRun 'apm update' to check for newer versions.info line.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com