feat(config): persist non-secret installer prefs for self-update (closes #667)#1915
Conversation
Persist bounded non-secret self-update installer preferences through apm config: the update channel and installer target directory. Self-update now reads those preferences while keeping credentials, tokens, mirror URLs, and executable settings out of persisted self-update config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a narrowly scoped, non-secret persistence mechanism for apm self-update installer defaults via apm config, plus prerelease channel support in the GitHub version lookup, and updates docs/tests to match the new behavior.
Changes:
- Introduces persisted config keys
self-update.channelandself-update.install-dir, with validation and unset support. - Updates
apm self-updateto apply config-backed installer environment defaults when invocation env vars are absent, and adds prerelease lookup via the GitHub releases list endpoint. - Adds unit tests and updates CLI documentation + apm-usage resources to document precedence and the “non-secret only” trust boundary.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_self_update_config.py | New tests covering env-vs-config precedence and prerelease release-list lookup. |
| tests/unit/test_config.py | Adds roundtrip tests for the new self-update config helpers. |
| tests/unit/test_config_command.py | Adds CLI routing tests for get/set/unset and rejects unsupported self-update.* keys on set. |
| src/apm_cli/utils/version_checker.py | Adds prerelease-capable lookup and shared tag normalization. |
| src/apm_cli/config.py | Adds typed config helpers for self_update_channel and self_update_install_dir. |
| src/apm_cli/commands/self_update.py | Reads effective channel + config-backed installer env defaults for self-update. |
| src/apm_cli/commands/config.py | Routes new keys and adds fail-closed behavior for unsupported self-update.* keys on set. |
| packages/apm-guide/.apm/skills/apm-usage/governance.md | Documents the user-config trust boundary and what must not be persisted. |
| packages/apm-guide/.apm/skills/apm-usage/commands.md | Updates command reference to include the new config keys and precedence. |
| docs/src/content/docs/reference/cli/self-update.md | Documents the new keys, precedence, and non-persistence of secrets/mirrors. |
| docs/src/content/docs/reference/cli/config.md | Adds the new config keys and their resolution order to the config reference. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 3
| if key.startswith("self-update."): | ||
| logger.error( | ||
| "self-update config only supports non-secret installer preferences: " | ||
| "self-update.channel, self-update.install-dir" | ||
| ) |
| if _channel != "stable": | ||
| logger.progress(f"Self-update channel: {_channel}") | ||
|
|
||
| _pinned = os.environ.get("VERSION", "") |
| tag_name = data.get("tag_name", "") | ||
| return _normalize_release_tag(tag_name) |
Bounds the prerelease release-list fetch, suppresses the default self-update channel in config summaries, improves the prerelease miss message, and adds regression coverage for malformed install-dir values. Addresses apm-review-panel follow-ups for PR #1915. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Suppresses unset self-update config defaults consistently, makes the prerelease recovery command copy-safe, and closes the documentation loop for reverting persisted self-update preferences. Addresses the final panel output-UX and docs follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 1 | Clean config triplet extension; future dispatch-table refactor can wait until more self-update keys exist. |
| CLI Logging Expert | 0 | 0 | 0 | Default and unset self-update values are now suppressed consistently in config summaries. |
| DevX UX Expert | 0 | 0 | 0 | Prerelease miss path now tells users how to return to stable explicitly. |
| Supply Chain Security Expert | 0 | 0 | 0 | Path validation and tests now guard malformed install-dir values without widening persisted config. |
| OSS Growth Hacker | 0 | 0 | 0 | Docs now include the set, use, and unset loop for self-update preferences. |
| Auth Expert | 0 | 0 | 0 | Auth surface remains untouched; no persisted credentials or token paths were introduced. |
| Doc Writer | 0 | 0 | 0 | Docs match the bounded key set and preserve the no-secrets boundary. |
| Test Coverage Expert | 0 | 0 | 0 | Targeted tests passed; malformed path guard was mutation-checked. |
| Performance Expert | 0 | 0 | 0 | Prerelease lookup now bounds GitHub release-list payload with per_page=5. |
B = highest-severity findings, R = recommended, N = nits. Counts are signal strength, not gates. The maintainer ships.
Architecture
flowchart TD
A["apm config set self-update.channel prerelease"] --> B["allowlisted config helper"]
C["apm config set self-update.install-dir PATH"] --> D["path validation + config helper"]
B --> E["config.json non-secret prefs"]
D --> E
F["apm self-update"] --> G{"env var present?"}
G -->|yes| H["invocation env wins"]
G -->|no| E
E --> I["installer env: channel/install-dir only"]
H --> I
I --> J["installer subprocess"]
K["tokens, mirrors, commands, args"] --> L["not persisted"]
Recommendation
Ready for maintainer review. The panel recommendation is to ship the current head after the required human review gate clears.
Folded in this run
- (panel) Bound prerelease release-list fetch with
per_page=5-- resolved in6ee00de7. - (panel) Added path validation and malformed install-dir regression coverage -- resolved in
6ee00de7. - (panel) Suppressed default
self-update.channelfrom config summaries -- resolved in6ee00de7. - (panel) Added channel-aware prerelease lookup failure guidance -- resolved in
6ee00de7. - (panel) Suppressed unset
self-update.install-dirfromapm config getsummaries -- resolved in22270c9d. - (panel) Added self-update docs revert steps and clarified the config example's why -- resolved in
22270c9d.
Copilot signals reviewed
- No Copilot inline comments were present after either fetch round. Copilot's top-level review was overview-only, with no actionable inline findings to fold.
Trust-surface boundary
No panel recommendation widened persisted config toward credentials, registry tokens, mirror URLs, commands, or args. That expansion remains out of scope for this PR and must continue to resolve through existing auth/env paths.
Regression-trap evidence
tests/unit/test_config.py::TestSelfUpdateInstallerConfig::test_install_dir_rejects_malformed_paths-- disabled the malformed path guards; test FAILED as expected; guards restored.
Lint contract
uv run --extra dev ruff check src/ tests/, uv run --extra dev ruff format --check src/ tests/, pylint R0801, and bash scripts/lint-auth-signals.sh all exited 0 before push.
CI
gh pr checks 1915 --repo microsoft/apm --watch observed all required checks green on head 025654194f8bddbd8819cfc3a60fca24162c9766.
Mergeability status
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #1915 | 0256541 |
ship_now | 2 | 6 | 0 | 2 | green | MERGEABLE | BLOCKED | pending required human review |
Convergence
2 outer iteration(s); 2 Copilot round(s). Final panel stance: ship_now. Ready for maintainer review.
Full per-persona findings
- Python Architect: no remaining in-scope architecture finding; future self-update dispatch-table refactor deferred until the key set grows beyond this PR.
- CLI Logging Expert: config output polish folded.
- DevX UX Expert: prerelease recovery wording folded.
- Supply Chain Security Expert: no persisted secret/executable/mirror expansion; path hardening folded.
- OSS Growth Hacker: docs loop folded.
- Auth Expert: no auth bypass or token persistence concern.
- Doc Writer: docs align with code.
- Test Coverage Expert: targeted tests passed; mutation-break evidence recorded.
- Performance Expert: release-list payload bound folded; broader transport optimizations are outside this PR.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
feat(config): persist non-secret installer prefs for self-update
TL;DR
This PR closes #667 by adding two bounded
apm configkeys for self-update installer defaults:self-update.channelandself-update.install-dir.apm self-updatenow reads those non-secret preferences, while credentials, registry tokens, mirror URLs, commands, and installer args stay out of persisted self-update config and continue through the existing auth/env paths.Important
Trust boundary: this intentionally persists only non-secret, non-executable installer preferences.
Problem (WHY)
apm config, with a hard scope limit: "Scope the first cut to non-secret, non-executable installer preferences only".self-update.*key instead of creating an open-ended installer schema.Approach (WHAT)
self_update_channelandself_update_install_dironly.apm config get/set/unset self-update.channelandself-update.install-dir.self-update.*keys with an actionable error.Implementation (HOW)
src/apm_cli/config.pysrc/apm_cli/commands/config.pyself-update.*keys.src/apm_cli/commands/self_update.pysrc/apm_cli/utils/version_checker.pytests/unit/*docs/...andpackages/apm-guide/...Diagram
Legend: the dashed nodes are new in this PR; env vars remain the highest-priority override.
flowchart LR subgraph Config[Config] C1[self-update.channel] C2[self-update.install-dir] C3[unsupported self-update keys] end subgraph Resolve[Self-update resolution] R1[env vars] R2[config fallback] R3[installer subprocess env] end subgraph Boundary[Trust boundary] B1[credentials and tokens] B2[mirror URLs] B3[commands and args] end C1 --> R2 C2 --> R2 R1 --> R3 R2 --> R3 C3 --> B1 C3 --> B2 C3 --> B3 classDef new stroke-dasharray: 5 5; class C1,C2,R2,R3 new;Trade-offs
prereleasechannel support because an update channel needs observable behavior; stable remains the default.self-update.*keys fail closed instead of being stored as raw config.Benefits
Validation
Command output
Scenario evidence
apm configcan set/get allowed installer keystests/unit/test_config.py,tests/unit/test_config_command.pyapm self-updatereads persisted prefstests/unit/test_self_update_config.py::test_update_installer_env_reads_non_secret_configtests/unit/test_self_update_config.py::test_update_installer_env_preserves_invocation_envtests/unit/test_config_command.py::test_set_rejects_secret_or_executable_self_update_keysHow to test
apm config set self-update.channel prereleaseandapm config get self-update.channel.apm config set self-update.install-dir ~/.local/binandapm config get self-update.install-dir.apm config set self-update.token secretand confirm it exits non-zero.apm self-update --checkwith and withoutAPM_SELF_UPDATE_CHANNELto verify env override behavior.Closes #667
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com