feat(cli): promote 'marketplace doctor' to top-level 'apm doctor' (+ workflow discoverability)#1539
Conversation
) Closes one of the four asks in #1537 by promoting the diagnostics verb to a top-level command and adding small discoverability hooks around the existing lifecycle commands. Tier 1 (this PR): * Promote 'apm marketplace doctor' to 'apm doctor'. The legacy invocation keeps working as a hidden deprecated alias that prints a one-line migration hint before delegating. * Add a 'Common workflows' epilog to 'apm --help' so init -> install -> outdated -> update -> doctor and the 'install --frozen && audit --ci' CI idiom are discoverable from the root help. * Add contextual error tips: AuthenticationError now points to 'apm doctor'; FrozenInstallError points to 'apm outdated' + 'apm update'. Five touch sites, one-line nudges each. * New Rosetta Stone guide 'Operating installed context' mapping npm/pnpm/uv/cargo/brew vocabulary to existing apm verbs (deps why, deps tree, outdated, install --frozen, audit --ci, doctor). Tier 2 deferred: 'apm status' summary view -- waits for a concrete shape that isn't already covered by deps tree + outdated + targets. Tier 3 declined: 'apm sync', 'apm sync --update', and 'apm check' are exact duplicates of 'install --frozen', 'update', and 'audit --ci' respectively (the 'check' name also collides with 'apm marketplace check'). Documented in the issue reply. The arch-invariant budget for commands/install.py is raised 2010 -> 2012 with justification matching the lockfile-UX precedent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Promotes the existing marketplace diagnostics flow into a discoverable top-level CLI command (apm doctor), while keeping apm marketplace doctor as a hidden, deprecated alias. This supports the installed-context workflow requested in #1537 and adds user-facing discoverability via root help text, targeted error tips, and a new docs guide.
Changes:
- Added top-level
apm doctorcommand that delegates to sharedrun_doctor()implementation. - Hid
apm marketplace doctorfrom marketplace help, but preserved it as a deprecated alias with a migration hint. - Added root
apm --help“Common workflows” epilog, contextual tips in install/update error handlers, new unit tests, and a new docs “Rosetta Stone” guide.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/install/test_architecture_invariants.py | Updates install.py LOC budget and justification text. |
| tests/unit/commands/test_doctor.py | Adds unit tests for command registration, help discoverability, alias behavior, and a diagnostics smoke run. |
| src/apm_cli/commands/update.py | Adds “Tip:” nudges for FrozenInstallError and AuthenticationError. |
| src/apm_cli/commands/marketplace/doctor.py | Extracts run_doctor() and converts legacy marketplace command into a hidden deprecated alias. |
| src/apm_cli/commands/marketplace/init.py | Removes doctor from marketplace authoring command listings. |
| src/apm_cli/commands/install.py | Adds “Tip:” nudges for auth and frozen-install drift paths. |
| src/apm_cli/commands/doctor.py | New top-level Click wrapper around run_doctor(). |
| src/apm_cli/cli.py | Registers doctor at top level and adds a “Common workflows” epilog to root help. |
| docs/src/content/docs/guides/operating-installed-context.md | New guide mapping common “installed context” questions to existing APM commands. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 7
| assert n <= 2012, ( | ||
| f"commands/install.py grew to {n} LOC (budget 2012). " | ||
| "Do NOT trim cosmetically -- engage the python-architecture skill " | ||
| "(.github/skills/python-architecture/SKILL.md) and propose an " |
There was a problem hiding this comment.
Good catch — fixed in 8c2e073. The path now points to .apm/skills/python-architecture/SKILL.md (the source of truth; .agents/ is the compiled mirror).
| # Deprecation hint is written to stderr; CliRunner merges streams by | ||
| # default in Click >= 8.2, so it shows up in result.output. | ||
| combined = result.output |
There was a problem hiding this comment.
Made the assertion stream-agnostic (combines stdout + stderr) in 8c2e073. The test was passing on Click 8.2.1 but only by luck; this version no longer depends on the merge default.
| _CLI_EPILOG = """\b | ||
| Common workflows: | ||
| apm init Scaffold a new project | ||
| apm install Install dependencies from apm.yml | ||
| apm install --frozen Reproduce lockfile exactly (CI-safe) | ||
| apm outdated See what's drifted from upstream | ||
| apm update Refresh refs and rewrite the lockfile | ||
| apm audit --ci Validate lockfile integrity for CI gates | ||
| apm doctor Diagnose environment problems | ||
| apm run <script> Execute a script from apm.yml |
There was a problem hiding this comment.
The '\b' here is a documented Click formatting directive that disables Click's epilog rewrapping (see the same pattern in src/apm_cli/commands/view.py lines 422 and 425). Without it, Click flows the eight workflow lines into one paragraph. I reformatted as concatenated strings (8c2e073) so the directive is more idiomatic, but kept the '\b' since dropping it visibly breaks the table layout. Treating it as a CLI rendering directive rather than user-facing output, matching the repo precedent.
| _rich_error(str(e)) | ||
| if e.diagnostic_context: | ||
| _rich_echo(e.diagnostic_context) | ||
| _rich_info("Tip: run 'apm doctor' to diagnose auth and connectivity.") |
| _rich_info("Tip: run 'apm doctor' to diagnose auth and connectivity.") | ||
| 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) | ||
| _rich_info("Tip: run 'apm outdated' to see what changed, then 'apm update'.") |
| _rich_error(str(e)) | ||
| for reason in e.reasons: | ||
| _rich_echo(reason) | ||
| _rich_info("Tip: run 'apm outdated' to see what changed, then 'apm update'.") | ||
| sys.exit(1) | ||
| except AuthenticationError as e: | ||
| _rich_error(str(e)) | ||
| if e.diagnostic_context: | ||
| _rich_echo(e.diagnostic_context) | ||
| _rich_info("Tip: run 'apm doctor' to diagnose auth and connectivity.") | ||
| sys.exit(1) |
| | Diagnose a broken environment | `apm doctor` | Aggregated pass/fail table: git, network, auth, gh CLI, and (if present) marketplace config. | | ||
| | Inspect the cache | `apm cache info` | Disk usage and location. `apm cache clean` removes everything; `apm cache prune --days N` is incremental. | | ||
| | Inspect resolved runtimes | `apm runtime status` | Active runtime and preference order. | | ||
| | Inspect resolved targets | `apm targets --all` | Which harnesses APM will deploy to. | |
There was a problem hiding this comment.
Updated in 8c2e073 — row now says 'apm targets' and mentions '--json --all' for the meta-target.
Folds 7 inline review comments: * tests/unit/install/test_architecture_invariants.py: correct skill path .github -> .apm/skills/python-architecture/SKILL.md (the pre-existing message pointed to a non-existent location). * tests/unit/commands/test_doctor.py: the deprecation-hint assertion now checks both stdout and stderr so it does not depend on Click 8.2's stream-separation default. * src/apm_cli/cli.py: reformat _CLI_EPILOG so the per-line layout survives Click's epilog rewrapping. Keeps the documented Click '\b' formatting marker (already used in src/apm_cli/commands/view.py lines 422 and 425) which is a CLI rendering directive, not user-facing output. * src/apm_cli/commands/install.py, src/apm_cli/commands/update.py: pass symbol='info' on the new _rich_info() tip lines so they render with the standard [i] prefix and match neighbouring usage. * docs/src/content/docs/guides/operating-installed-context.md: document 'apm targets' as the default invocation; mention '--json --all' for the agent-skills meta-target. Arch budget for install.py raised 2012 -> 2014 to absorb ruff's multi-line reformat of the now-keyword-arg _rich_info() call. Justification kept inline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(tests): skip POSIX execute-bit assertions on Windows
Two unit tests asserted st_mode & 0o111 == 0o111, which fails on
Windows because NTFS does not honor POSIX execute bits. Guard both
with pytest.mark.skipif(sys.platform == 'win32'), matching the
existing convention used elsewhere in the suite.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(scripts): guard empty PYTEST_EXTRA_ARGS array under bash 3.2
scripts/test-integration.sh runs under `set -euo pipefail`. On macOS
the default /bin/bash is 3.2, where expanding an empty array with a
bare "${arr[@]}" raises an unbound-variable error. Local integration
runs (PYTEST_EXTRA_ARGS unset) aborted before pytest with
'extra_args[@]: unbound variable'. Use the ${arr[@]+"${arr[@]}"}
guard so the empty-array expansion is safe; CI behaviour (with
PYTEST_EXTRA_ARGS set) is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore: release v0.16.1
Roll the [Unreleased] changelog into a dated 0.16.1 block and bump
pyproject.toml + uv.lock. Adds the previously-missing user-facing
entries for #1539 (apm doctor), #1566, #1569, #1567, #1553, #1552,
and #1538 surfaced by enumerating merged PRs since v0.16.0.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* ci: parallelise macOS release integration tests to fix 20min timeout
The v0.16.1 release pipeline failed on the macOS x86_64 (Intel) build:
the consolidated job's "Run integration tests" step hit its 20-minute
timeout at ~61% progress. The tests were passing -- the slower Intel
runner simply could not finish the full suite serially in time, and the
arm64 runner was also near the edge.
Unlike ci-integration.yml, which shards the suite across four runners,
the release workflow runs the whole integration suite on a single
scarce macOS runner. Parallelise it in-process with xdist (-n 2,
matching ci-integration's per-shard width to bound shared-PAT API
concurrency) using --dist loadgroup so the home_env xdist_group markers
keep HOME-mutating tests serialized on one worker. Also raise the step
timeout to 30 minutes for headroom on the slow Intel runner.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TL;DR
Partial fix for #1537. Promotes the diagnostics verb to a top-level command, adds discoverability hooks around the existing lifecycle, and writes a Rosetta Stone guide that maps npm/pnpm/uv/cargo/brew vocabulary to existing APM verbs. Declines the renames that would create alias debt; defers
apm statusuntil a concrete shape emerges.Decision (three-tier disposition of #1537)
apm syncapm install --frozen, which is the same idiom contributors learn fromnpm ci/pnpm install --frozen-lockfile/uv sync --frozen. Addingsyncmakes two names for one thing.apm sync --updateapm update.apm checkapm audit --ci(the CI gate). The namecheckalso collides with the existingapm marketplace check.apm statusapm deps tree+apm outdated+apm targets. Will ship if a concretestatusoutput emerges that isn't covered by composing those.apm doctormarketplace.Full reasoning in the issue reply (four-panel synthesis: devx-pro, devx-con, oss-growth, apm-ceo).
What ships
apm doctor— top-level command, thin wrapper around the existing 7-check diagnostic suite (git on PATH, github.com reachability, AuthResolver token, gh CLI, marketplace config, format coverage, duplicate names, version alignment).apm marketplace doctorstill works but is hidden frommarketplace --helpand prints[!] 'apm marketplace doctor' is deprecated; use 'apm doctor' instead.before delegating. Zero behavior regression for existing scripts/CI.apm --help"Common workflows" epilog — surfaces the lifecycle (init -> install -> outdated -> update -> doctor) and the CI idiom (apm install --frozen && apm audit --ci) from the root help.AuthenticationErrornow points toapm doctor;FrozenInstallErrorpoints toapm outdated+apm update. Five touch sites ininstall.py(x2) andupdate.py(x2 + reused), one_rich_infoline each.docs/src/content/docs/guides/operating-installed-context.mdmaps "I want to..." questions to existing verbs including the under-discoveredapm deps why,apm deps tree,apm install --frozen, andapm audit --ci.What deliberately does not ship
apm status— deferred until a concrete output is articulated. Mentioned as a follow-up in the issue reply.run_doctor()callable inmarketplace/doctor.pyis the seam for follow-up PRs to add domains incrementally.Validation evidence
Lint mirror (canonical contract):
Tests:
15790 passed, 1 skipped, 21 xfailed(fulltests/unitrun, ~2:30). Six new tests intests/unit/commands/test_doctor.pycover registration, root-help discovery, the workflows epilog, marketplace-help hiding, deprecation hint, and a smoke run of diagnostics.The
commands/install.pyarch-invariant budget is raised 2010 -> 2012 with a justification comment matching the lockfile-UX precedent (#1203). Two lines added are pure Click-handler glue (_rich_infoper except branch); no new logic.How to test
Touched files
src/apm_cli/commands/doctor.py(new) — top-level Click wrappersrc/apm_cli/commands/marketplace/doctor.py— extractedrun_doctor()callable; legacy command is now a hidden deprecated aliassrc/apm_cli/commands/marketplace/__init__.py— removed"doctor"fromMarketplaceGroup._authoring_commandssrc/apm_cli/cli.py— wired top-level command + added_CLI_EPILOGsrc/apm_cli/commands/install.py— two error-hint nudgessrc/apm_cli/commands/update.py— two error-hint nudgestests/unit/commands/test_doctor.py(new) — 6 teststests/unit/install/test_architecture_invariants.py— budget 2010 -> 2012 with justificationdocs/src/content/docs/guides/operating-installed-context.md(new) — Rosetta StoneRefs #1537 (partial; defer
apm statusfollow-up)