fix(uninstall): accept Windows absolute paths as local packages (re-tag v0.14.1) by danielmeppiel · Pull Request #1413 · microsoft/apm

danielmeppiel · 2026-05-20T07:25:03Z

Why

The v0.14.1 release pipeline failed Windows-only on two copilot-app uninstall e2e tests, and no GitHub Release was published — so v0.14.1 never shipped a binary to users. Rather than cut v0.14.2, this PR rolls the fix into v0.14.1 itself; after merge we delete and re-tag v0.14.1 on the new merge commit so the release pipeline can publish the real v0.14.1.

Root cause

_validate_uninstall_packages in src/apm_cli/commands/uninstall/engine.py used if "/" not in package: to detect bare marketplace slugs. Windows absolute paths (C:\Users\...\my-pkg) contain only \ and therefore hit the "Invalid package format" branch. POSIX paths contain / so they bypassed the guard — that's why it was Linux/macOS-green and Windows-red.

The validator rejected the path before the DependencyReference parse path could run, so the uninstall exited "successfully" with empty work and the deployed copilot-app DB row leaked.

Fix

engine.py:241-242 — consult DependencyReference.is_local_path() before bailing out. Local paths fall through to the existing parse/match-against-lockfile flow.
cli.py:219-225 — replace silent except Exception: pass around _sync_integrations_after_uninstall with logger.warning(...). Surfaces previously-swallowed cleanup errors. Wasn't the trigger here but is a permanent UX improvement.
CHANGELOG.md — single Fixed entry under the existing [0.14.1] section.

Security review

Supply-chain reviewed: APPROVE, no nits. Three independent gates still apply:

The CLI arg is only matched against the lockfile via get_identity(); it is never a deletion target.
DependencyReference.get_install_path() reduces local-package paths to apm_modules/_local/<basename>/ and calls ensure_path_within(apm_modules_dir).
safe_rmtree re-checks containment with ensure_path_within before recursive delete.

So even apm uninstall C:\Windows\System32 could at worst delete apm_modules/_local/System32/, and only if the user's own lockfile already claimed that local_path.

Tests

New regression tests/unit/test_uninstall_engine_helpers.py::test_windows_absolute_path_not_rejected_as_invalid_format covers C:\... and .\... inputs.
The two e2e tests (tests/unit/install/test_install_target_copilot_app_e2e.py) now dump install/uninstall output and the lockfile on assertion failure for faster future triage.

Validation

macOS: 55 related tests pass; lint clean (ruff check + ruff format --check).
All 5 platform Build & Test jobs PASS on this branch (build-release.yml run 26148343587), including Windows x64.

Post-merge release plan

Delete tag v0.14.1 locally and on origin.
Re-tag v0.14.1 on the merge commit.
Push tag — release pipeline publishes the actual v0.14.1 binaries.

The Windows-only e2e failures in test_install_target_copilot_app_e2e.py left the copilot-app DB row in place after `apm uninstall` because the `_sync_integrations_after_uninstall` call was wrapped in a bare `except Exception: pass`. Any platform-specific failure (file lock, path encoding, etc.) was hidden, and the user saw a successful uninstall while state on disk was inconsistent. Replace the silent swallow with a warning log so the failure mode is visible both in CI logs and to end users, and expand the two failing e2e tests to dump the verbose CLI output and lockfile contents on assertion failure so the underlying Windows error surfaces in the next CI run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Root cause of the v0.14.1 Windows-only release failure: the package-arg validation loop in `src/apm_cli/commands/uninstall/engine.py` did `if "/" not in package`, which on Windows treats absolute local paths (`C:\\Users\\runneradmin\\AppData\\Local\\Temp\\...\\my-pkg`) as malformed marketplace refs and rejects them with "Invalid package format: ... Use 'owner/repo' or 'plugin-name@marketplace' format.". The uninstall exits 0, leaves the package in apm.yml, never reaches the copilot-app integration cleanup, and the DB row leaks. macOS and Linux were unaffected because their POSIX paths contain `/` and bypass the guard. Fix: also consult `DependencyReference.is_local_path()` (which already recognises Windows drive-letter paths, `.\\` and `..\\` shorthand, and POSIX paths) before bailing out. Add a unit-level regression in `tests/unit/test_uninstall_engine_helpers.py` so the validator explicitly accepts `C:\\...` and `.\\...` arguments and the "Invalid package format" error path can never silently come back. Keep the user-visible improvements made while diagnosing this: - `commands/uninstall/cli.py` now logs (warning) when the post-uninstall integration sync raises, instead of swallowing every exception with bare `except: pass`. The previous silent path is exactly why this Windows failure was invisible. - The two copilot-app e2e tests dump install + uninstall output and the post-uninstall lockfile on failure, so any future Windows-only cleanup regression surfaces with full context in CI logs. Bump version to 0.14.2 and add CHANGELOG entry. v0.14.1 stays tagged but its Windows binary failed to ship; this PR rolls forward. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Improves Windows uninstall diagnostics and fixes a Windows-specific uninstall validation edge case so integration cleanup failures are no longer silently hidden and can be debugged from CI logs.

Changes:

Treat Windows local paths as valid uninstall arguments by using local-path detection in _validate_uninstall_packages.
Surface integration-sync exceptions during uninstall via warning logs, and enrich failing e2e assertions with CLI + lockfile dumps.
Bump version/changelog for 0.14.2.

Show a summary per file

File	Description
`src/apm_cli/commands/uninstall/engine.py`	Fixes Windows path handling in uninstall package validation.
`src/apm_cli/commands/uninstall/cli.py`	Stops swallowing integration cleanup errors; logs warnings instead.
`tests/unit/test_uninstall_engine_helpers.py`	Adds regression test for Windows absolute/relative path handling.
`tests/unit/install/test_install_target_copilot_app_e2e.py`	Adds verbose uninstall output + lockfile dump on failure for CI debugging.
`pyproject.toml`	Bumps package version to `0.14.2`.
`CHANGELOG.md`	Documents the Windows uninstall fix and improved error surfacing.
`apm.lock.yaml`	Large removal of `local_deployed_files` and hash metadata.

Copilot's findings

Files reviewed: 7/8 changed files
Comments generated: 4

+        except Exception as _sync_err:
+            # Surface why integration cleanup failed instead of swallowing
+            # silently. Previously a bare `except: pass` here masked
+            # Windows-only failures where the DB row was never deleted on
+            # `apm uninstall --target copilot-app`.
+            logger.warning(f"Integration cleanup skipped: {type(_sync_err).__name__}: {_sync_err}")
+            logger.verbose_detail(
+                "Some integrated files may remain. Run `apm install --force` to resync."
+            )


+            # silently. Previously a bare `except: pass` here masked
+            # Windows-only failures where the DB row was never deleted on
+            # `apm uninstall --target copilot-app`.
+            logger.warning(f"Integration cleanup skipped: {type(_sync_err).__name__}: {_sync_err}")


+        # A package arg is either: (a) a marketplace single-token slug, (b)
+        # an `owner/repo` slug, or (c) a local filesystem path. The legacy
+        # check below only allowed (a) when there is no `/`, but Windows
+        # absolute paths use backslashes (e.g. `C:\Users\...\my-pkg`) and
+        # therefore have no `/` either -- they would be wrongly rejected
+        # as "Invalid package format" and the DB row for any deployed
+        # copilot-app workflow would leak. Use the canonical local-path
+        # detector instead so paths fall through to DependencyReference
+        # parsing on every platform.
+        is_local = DependencyReference.is_local_path(package)
+        if "/" not in package and not is_local:


 lockfile_version: '1'
 generated_at: '2026-04-21T21:45:34.516938+00:00'
 dependencies: []


v0.14.1 release pipeline failed on Windows before any GitHub Release was published, so no binary ever shipped. Roll the uninstall fix into v0.14.1 itself rather than cutting v0.14.2, and re-tag v0.14.1 on the new merge commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The previous fix commit accidentally captured an apm.lock.yaml diff from a local 'apm uninstall' test run. Restore the lockfile to main state; the PR should only contain the bugfix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- 'Integration cleanup skipped' -> 'Integration cleanup failed': the branch fires only when the cleanup was attempted and raised, so 'skipped' was misleading. - Append traceback under verbose_detail so platform-specific (Windows) exceptions are debuggable without spamming default output. - Tighten the validator comment to describe the real marketplace ref format (name@marketplace[#ref]) instead of 'single-token slug'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel marked this pull request as ready for review May 20, 2026 07:31

danielmeppiel requested a review from sergio-sisternes-epam as a code owner May 20, 2026 07:31

Copilot AI review requested due to automatic review settings May 20, 2026 07:31

danielmeppiel changed the title ~~debug(uninstall): surface integration sync errors to expose Windows failure~~ fix(uninstall): accept Windows absolute paths as local packages May 20, 2026

Copilot started reviewing on behalf of danielmeppiel May 20, 2026 07:37 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

danielmeppiel changed the title ~~fix(uninstall): accept Windows absolute paths as local packages~~ fix(uninstall): accept Windows absolute paths as local packages (re-tag v0.14.1) May 20, 2026

Daniel Meppiel and others added 2 commits May 20, 2026 09:59

danielmeppiel merged commit 90edce4 into main May 20, 2026
9 checks passed

danielmeppiel deleted the fix-windows-copilot-app-uninstall branch May 20, 2026 10:44

This was referenced May 21, 2026

feat(copilot-app): hybrid WS-IPC + SQLite project scoping for workflows #1431

Merged

docs: backfill apm-usage and consolidate registry guides (v0.14->v0.15 drift sweep) #1511

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(uninstall): accept Windows absolute paths as local packages (re-tag v0.14.1)#1413

fix(uninstall): accept Windows absolute paths as local packages (re-tag v0.14.1)#1413
danielmeppiel merged 5 commits into
mainfrom
fix-windows-copilot-app-uninstall

danielmeppiel commented May 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

danielmeppiel commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Root cause

Fix

Security review

Tests

Validation

Post-merge release plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danielmeppiel commented May 20, 2026 •

edited

Loading