Skip to content

feat: Add build-time policy to disable self-update in packaged builds#675

Merged
danielmeppiel merged 6 commits intomicrosoft:mainfrom
melund:update-policy
Apr 21, 2026
Merged

feat: Add build-time policy to disable self-update in packaged builds#675
danielmeppiel merged 6 commits intomicrosoft:mainfrom
melund:update-policy

Conversation

@melund
Copy link
Copy Markdown
Contributor

@melund melund commented Apr 11, 2026

Description

Add a build-time self-update policy so package-manager distributions of apm (for example pixi or brew) can disable apm update and show a custom guidance message instead.

This is useful for for example the conda-forge packaged version of apm, which I have added here:
https://github.com/conda-forge/apm-cli-feedstock

In such cases apm should not update it self.

The PR creates a update_policy module which package mangers can patch. It wires the policy into the update command and startup update notification path.

The PR also updates docs to explain distribution-managed update behavior. It also adds unit tests for the new disabled-policy flow.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Copilot AI review requested due to automatic review settings April 11, 2026 14:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a build-time policy hook that allows packaged/distribution builds (brew/pixi/conda, etc.) to disable apm update and provide distributor-specific update guidance, while also adjusting the startup update notification behavior.

Changes:

  • Introduces src/apm_cli/update_policy.py with patchable constants and helper accessors for self-update enablement and messaging.
  • Wires the policy into apm update (early exit with guidance) and into _check_and_notify_updates() (skip update notifications when disabled).
  • Updates CLI reference docs and adds unit tests covering the disabled-policy flows.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/unit/test_update_command.py Adds a unit test ensuring apm update exits successfully and skips network/installer when policy disables self-update.
tests/unit/test_command_helpers.py Adds a unit test ensuring startup update checks are skipped when self-update is disabled.
src/apm_cli/update_policy.py New centralized module for build-time self-update policy + guidance message helpers.
src/apm_cli/commands/update.py Enforces the policy in the update command by short-circuiting when disabled.
src/apm_cli/commands/_helpers.py Skips update notifications when disabled; uses a helper to render the update hint consistently.
docs/src/content/docs/reference/cli-commands.md Documents distribution-managed update behavior and the skipped startup notification behavior.

Comment thread src/apm_cli/update_policy.py Outdated
Comment thread src/apm_cli/update_policy.py
Comment thread src/apm_cli/update_policy.py Outdated
Comment thread docs/src/content/docs/reference/cli-commands.md
@melund
Copy link
Copy Markdown
Contributor Author

melund commented Apr 11, 2026

@microsoft-github-policy-service agree

Comment thread docs/src/content/docs/reference/cli-commands.md Outdated
Copy link
Copy Markdown
Collaborator

@sergio-sisternes-epam sergio-sisternes-epam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @melund -- great use case for conda-forge and other distribution-managed builds. The overall design is clean and well-scoped.

One change requested: the bool() coercion on SELF_UPDATE_ENABLED can silently defeat the policy when packagers use string substitution during builds. See the inline comment for a one-line fix using identity check (is True) that fails safe.

Comment thread src/apm_cli/update_policy.py Outdated
@melund
Copy link
Copy Markdown
Contributor Author

melund commented Apr 12, 2026

Do you want me to rebase, or can you just squash-merge the PR?

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Do you want me to rebase, or can you just squash-merge the PR?

@melund we need you to update and resolve conflicts

@melund
Copy link
Copy Markdown
Contributor Author

melund commented Apr 18, 2026

@danielmeppiel Done

@danielmeppiel danielmeppiel added CI/CD and removed CI/CD labels Apr 19, 2026
Copy link
Copy Markdown
Collaborator

@danielmeppiel danielmeppiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APM Review Panel verdict: APPROVE

Ran this PR through the full panel (architecture, CLI logging UX, DevX UX, supply-chain security, growth, strategic). All seven personas land in the same place: merge it.

Per-persona findings

Python Architect — APPROVE

  • update_policy.py is 51 lines, single responsibility, clean. The "constants patchable by packagers at build time" pattern is industry-standard (pip, setuptools, gh CLI).
  • is_self_update_enabled() uses SELF_UPDATE_ENABLED is True — strict identity, fails closed if a packager mistypes the value. Defensive.
  • _is_printable_ascii guard on the distributor message keeps the project's hard ASCII rule intact even when third parties patch the constant.

CLI Logging UX — APPROVE (one note)

  • update.py correctly routes the disabled message through CommandLogger.warning. No direct _rich_* in the command body.
  • Non-blocking note: when self-update is disabled, _check_and_notify_updates early-returns before check_for_updates(), so packaged-build users get zero "newer version available" signal from APM. Defensible (avoids pointing at a no-op apm update), but a follow-up could surface "vX.Y.Z available; update via your package manager" without the misleading command.

DevX UX — STRONG APPROVE

  • Textbook package-manager etiquette pattern. Every mature CLI distributed via brew / apt / conda does exactly this (kubectl, gh, awscli, terraform). Letting apm update clobber a conda/brew-managed install would be the worst possible UX.
  • Distributor-customizable message (e.g. "Update with: pixi update apm-cli") gives users an exact next action.
  • apm update help and cli-commands.md both updated.

Supply Chain Security — APPROVE (net positive)

  • Zero new attack surface: read-only constants, no I/O, no auth.
  • ASCII guard blocks ANSI / terminal-control injection through the distributor message channel.
  • A malicious packager could already patch arbitrary source — this gives them a narrow, documented API instead of letting them rewrite update.py directly. Net improvement.
  • Reduces attack surface in distributor builds: disabled policy means no remote install.sh / install.ps1 download + execute path.
  • Hygiene follow-up (non-blocking): a CI sanity check that upstream tarballs ship with SELF_UPDATE_ENABLED = True would prevent an accidental flip.

OSS Growth — STRONG SIGNAL

  • External contributor packaged APM for conda-forge (apm-cli-feedstock) themselves and is now upstreaming the polish to make it work properly. Platonic ideal of an OSS contribution funnel.
  • conda-forge unlocks the data-science / scientific-Python audience APM was not reaching.
  • Merging visibly signals "we welcome distributor packaging" — invites pixi, nix, scoop, AUR follow-ons.

CEO — APPROVE with one small ask

  • Tiny PR (100 LOC, 7 files), focused, tested, docs + packages/apm-guide/.apm/skills/apm-usage/commands.md already synced. Author respected project conventions on a first contribution — that's not common.
  • Pre-merge ask (not blocking enough to bounce — a maintainer can add if you'd rather not respin): a CHANGELOG entry under ## [Unreleased] -> Added, e.g.:

    Added build-time update_policy module so package-manager distributions can disable apm update and show distributor-specific guidance instead. Used by the conda-forge apm-cli package. -- by @melund (#675)

Bottom line

Real downstream packager need, defensive validation, no new attack surface, opens a packaging channel APM doesn't have to operate. Sitting on it discourages the next packager; merging rewards the funnel. Thanks for the contribution and for maintaining the conda-forge feedstock.

@danielmeppiel danielmeppiel added this pull request to the merge queue Apr 21, 2026
@danielmeppiel danielmeppiel mentioned this pull request Apr 21, 2026
7 tasks
Merged via the queue into microsoft:main with commit fe9abbe Apr 21, 2026
11 checks passed
danielmeppiel added a commit that referenced this pull request Apr 21, 2026
- Bump version to 0.9.0 in pyproject.toml and uv.lock
- Move CHANGELOG [Unreleased] entries into [0.9.0] - 2026-04-21
- Add fresh empty [Unreleased] header
- Consolidate the two scattered ### Changed subsections into one
- Backfill missing entries: build-time self-update policy (#675),
  APM Review Panel skill instrumentation (#777)
- Backfill PR refs on a few entries that referenced issue numbers
- Promote the previously-orphaned 'apm install --mcp' / VS Code adapter /
  init Next Steps lines to consistent (#PR) attribution
- Remove stray blank line inside the ### Added block
- Credit external contributors inline (@arika0093 #700, @joostsijm #675,
  @edenfunf #788)

Highlights of 0.9.0:

BREAKING CHANGES
- MCP entry validation hardened (#807)
- Strict-by-default transport selection (#778)
- Whitespace stdio MCP commands now rejected at parse time (#809)

ADDED
- apm install --mcp for declarative MCP server addition (#810)
- --registry flag and MCP_REGISTRY_URL for custom MCP registries (#810)
- HTTP-dependency support via --allow-insecure dual opt-in (#700)
- apm install --ssh / --https flags for transport selection (#778)
- Multi-target apm.yml + --target flag (#628)
- Marketplace UX: view, outdated, validate (#514)
- Build-time self-update policy for package-manager distros (#675)
- APM Review Panel + 4 specialist personas (#777)

This PR is release machinery and is intentionally excluded from the
[0.9.0] section per .github/instructions/changelog.instructions.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants