Skip to content

fix(self-update): improve update UX on macOS#1872

Merged
danielmeppiel merged 1 commit into
microsoft:mainfrom
nadav-y:feat/default-registry-cli-routing
Jun 25, 2026
Merged

fix(self-update): improve update UX on macOS#1872
danielmeppiel merged 1 commit into
microsoft:mainfrom
nadav-y:feat/default-registry-cli-routing

Conversation

@nadav-y

@nadav-y nadav-y commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Two small quality-of-life fixes to the apm update flow on Unix.

Changes

Fix -e garbage output on macOS during self-update

_get_installer_run_command was invoking install.sh with /bin/sh. On macOS,
/bin/sh is bash 3.2 running in POSIX compatibility mode, where the echo builtin
does not recognize the -e flag and prints it literally — so every colored output
line printed with echo -e "..." appeared prefixed with -e in the terminal.
Switching to bash (via shutil.which("bash"), falling back to /bin/bash,
raising FileNotFoundError if neither exists) respects the script's own shebang
and fixes the output.

Show a progress bar during binary download

install.sh used curl --silent --show-error, which suppresses all output including the progress bar. Replacing --silent with --progress-bar gives users visual feedback during the download without adding verbose byte-count noise.

Notes

  • Windows (install.ps1) is unaffected — Invoke-WebRequest renders its own progress bar natively
  • Unit and integration tests for _get_installer_run_command updated to match new behavior

Copilot AI review requested due to automatic review settings June 21, 2026 09:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Improves the Unix self-update experience by running the installer script under bash (to avoid macOS /bin/sh echo-flag behavior) and by showing a curl progress bar during downloads.

Changes:

  • Switch _get_installer_run_command() to prefer bash on Unix.
  • Update unit + integration tests to expect bash-based execution.
  • Replace curl --silent with curl --progress-bar for the public download path in install.sh.

Reviewed changes

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

File Description
src/apm_cli/commands/self_update.py Updates the Unix installer runner to use bash.
tests/unit/test_update_command.py Adjusts helper tests for the new bash selection behavior.
tests/integration/test_commands_config_coverage.py Updates integration coverage assertion to require bash.
install.sh Enables a visible progress bar for the initial unauthenticated download.

Comment thread src/apm_cli/commands/self_update.py Outdated
Comment thread tests/unit/test_update_command.py
Comment thread install.sh
@nadav-y nadav-y force-pushed the feat/default-registry-cli-routing branch from 73a3727 to e25d228 Compare June 21, 2026 13:25
@nadav-y

nadav-y commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator Author

This is how the old code looked like:

~ apm update
[!] 'apm update' refreshes APM dependencies. To update the CLI binary, use 'apm self-update'. Forwarding for back-compat (deprecated).
[i] Current version: 0.20.0
[>] Checking for updates...
[*] Latest version available: 0.21.0
[>] Downloading and installing update...
[*] Running installer...
-e
+--------------------------------------------------------------+
|                         APM Installer                        |
|              The NPM for AI-Native Development               |
+--------------------------------------------------------------+
-e
-e Detected platform: darwin-arm64
-e Target binary: apm-darwin-arm64.tar.gz
-e Note: Will need sudo permissions to install to /usr/local/bin
-e Fetching latest release information...
-e Latest version: v0.21.0
-e Download URL: https://github.com/microsoft/apm/releases/download/v0.21.0/apm-darwin-arm64.tar.gz
-e Downloading APM...
-e [+] Download successful
-e Extracting binary...
-e [+] Extraction successful
-e Testing binary...
-e [+] Binary test successful
-e Installing APM CLI to /usr/local/bin...
-e [+] APM installed successfully!
-e Version: Agent Package Manager (APM) CLI version 0.21.0 (975f8f0)
-e Location: /usr/local/bin/apm -> /usr/local/lib/apm/apm

-e Installation complete!

-e Quick start:
  apm init my-app          # Create a new APM project
  cd my-app && apm install # Install dependencies
  apm run                  # Run your first prompt

-e Documentation: https://github.com/microsoft/apm
-e Need help? Create an issue at https://github.com/microsoft/apm/issues
[*] Successfully updated to version 0.21.0!

New one doesn't have the -e prefix, plus (single line)

Downloading APM...
####################################################################################################################################################### 100.0%

Two QoL improvements to the Unix self-update flow:

- Invoke install.sh with bash instead of /bin/sh. On macOS, /bin/sh is
  bash 3.2 running in POSIX compatibility mode, where the echo builtin
  does not recognize the -e flag and prints it literally — prefixing
  every colored output line with `-e`. Using bash (via shutil.which with
  /bin/bash fallback, raising FileNotFoundError if neither exists) respects
  the script's shebang and produces clean output.

- Replace curl --silent with --progress-bar across all download paths in
  install.sh (unauthenticated, GitHub API, and direct URL with auth) so
  users get visual feedback during the binary download.

Windows (install.ps1) is unaffected: Invoke-WebRequest renders its own
progress bar natively and PowerShell is already invoked explicitly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nadav-y nadav-y force-pushed the feat/default-registry-cli-routing branch from e25d228 to 9b93b2c Compare June 21, 2026 13:30
@danielmeppiel danielmeppiel added this pull request to the merge queue Jun 25, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 25, 2026
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_now

Bounded UX fix: bash-over-sh and progress bar for install script.

cc @nadav-y @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

PR #1872 is a clean, bounded UX improvement. The switch from sh to bash resolves well-documented macOS echo behavior divergence, and the progress bar replaces what was previously a silent download experience. All panelists aligned on the change being low-risk and user-facing. Test coverage for bash resolution is present and passing. The optional nits raised (install-bash hint for systems without bash, non-TTY progress noise, changelog entry) are stylistic and do not represent material gaps or user-facing regressions. No specialist disagreements require arbitration.

Aligned with: multi-harness/multi-host: bash targeting improves predictability across macOS and Linux hosts where bash is ubiquitous; OSS community-driven: improves first-run experience for new adopters; pragmatic as npm: progress feedback during install matches modern package-manager expectations.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 2 Clean, bounded change. bash requirement is correct for install.sh semantics; fallback chain is well-structured.
CLI Logging Expert 0 0 2 Progress bar swap is a clear UX win; missing-bash messaging could be friendlier later.
DevX UX Expert 0 1 1 Interactive download feedback improves self-update UX; optional missing-bash hint can wait.
Supply Chain Security Expert 0 0 1 Low-risk changes; curl auth headers and URLs are unchanged.
OSS Growth Hacker 0 0 1 Progress bar on downloads is a small but meaningful first-run UX win.
Auth Expert 0 0 0 No auth surface changed; token headers and credential resolution are unchanged.
Doc Writer 0 0 0 No documentation drift found for self-update shell choice or download progress.
Test Coverage Expert 0 0 0 Bash-resolution change has unit tests for all three branches plus updated integration assertion.

B = highest-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Architecture

classDiagram
    class self_update_module {
        +_get_installer_run_command(script_path) list
        +self_update(check) None
    }
    class install_sh {
        +download_binary()
    }
    class shutil {
        +which(name) str or None
    }
    class os_path {
        +exists(path) bool
    }
    self_update_module ..> shutil : resolves bash
    self_update_module ..> os_path : fallback /bin/bash
    self_update_module ..> install_sh : runs script
    install_sh ..> curl : downloads with progress bar
Loading
flowchart TD
    A[apm self-update] --> B{Windows?}
    B -- yes --> C[PowerShell script]
    B -- no --> D[Resolve bash]
    D --> E[Run install.sh]
    E --> F[curl --progress-bar download]
    F --> G[Install binary]
Loading

Recommendation

Ship. No material follow-ups remain for this PR.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both silent.

CI

All PR checks pass: Lint, CodeQL, Merge Gate, NOTICE Drift Check, Spec conformance gate, Build & Test Shard 1 (Linux), Build & Test Shard 2 (Linux), PR Binary Smoke (Linux), APM Self-Check, Coverage Combine (Linux), and license/cla are green (0 CI fix iterations).

Mergeability status

Captured from gh pr view 1872 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last panel pass.

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#1872 9b93b2c ship_now 1 0 0 1 green MERGEABLE CLEAN

Convergence

1 outer iteration; 1 Copilot round. Final panel stance: ship_now.

Ready for maintainer review.


Full per-persona findings

Python Architect

  • [nit] src/apm_cli/commands/self_update.py:13 -- shutil import usage is fine after this PR. PR adds a second shutil.which usage; no action needed.
  • [nit] src/apm_cli/commands/self_update.py:117 -- Consider debug logging the resolved bash path in a future diagnostics pass.

CLI Logging Expert

  • [nit] src/apm_cli/commands/self_update.py:116 -- FileNotFoundError message omits an actionable install-bash hint.
  • [nit] install.sh:438 -- curl progress output can be noisy in non-TTY logs; acceptable for this interactive UX change.

DevX UX Expert

  • [recommended] src/apm_cli/commands/self_update.py -- Missing-bash error could provide one concrete next action, but this is additive polish.
  • [nit] install.sh -- Non-TTY progress noise can be revisited if automation users report it.

Supply Chain Security Expert

  • [nit] src/apm_cli/commands/self_update.py -- PATH-based bash lookup is environment-dependent; practical risk is limited in a user-invoked self-update.

OSS Growth Hacker

  • [nit] CHANGELOG.md -- A future release note could call out the progress-bar UX win.

Auth Expert

No findings.

Doc Writer

No findings.

Test Coverage Expert

No findings.

Performance Expert -- inactive

Changed files do not touch cache, resolver, materialization, install pipeline, or performance-claim surfaces.

This panel is advisory. It does not affect mergeability. Re-apply the panel-review label after addressing feedback to re-run.

@danielmeppiel danielmeppiel added this pull request to the merge queue Jun 25, 2026
Merged via the queue into microsoft:main with commit 3e8633f Jun 25, 2026
13 checks passed
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.

3 participants