feat: add azd tool uninstall command with per-host skill support#8794
Conversation
…rina/azure-dev into hemarina/azd-tool-uninstall
…na/azd-tool-uninstall
…rina/azure-dev into hemarina/azd-tool-uninstall
…rina/azure-dev into hemarina/azd-tool-uninstall
There was a problem hiding this comment.
Pull request overview
Adds the missing azd tool uninstall subcommand to complete the managed tool lifecycle, including special handling for skill tools that are installed via agent-host CLIs (per-host uninstall support) and corresponding telemetry/docs updates.
Changes:
- Introduces
azd tool uninstallwith--all,--dry-run, and skill-only--hostsupport (including JSON output). - Extends the tool manifest and installer/manager layers to support uninstall across package managers, explicit uninstall commands, direct-download artifacts, and per-host skill removal + verification.
- Updates telemetry coverage tests, telemetry reference docs, the metrics-audit matrix, and regenerated usage/Fig spec snapshots.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/specs/metrics-audit/feature-telemetry-matrix.md | Adds tool uninstall to the command→telemetry inventory/matrix. |
| docs/reference/telemetry-data.md | Updates tool-management telemetry field descriptions to include uninstall semantics. |
| cli/azd/pkg/tool/manifest.go | Adds uninstall metadata (PluginUninstallCommand, UninstallCommand) to the tool manifest model + populates for skills and azd extensions. |
| cli/azd/pkg/tool/manager.go | Adds UninstallTools orchestration, including skill-first ordering for --all / batch scenarios. |
| cli/azd/pkg/tool/manager_test.go | Unit tests for UninstallTools delegation, option forwarding, batching behavior, and ordering. |
| cli/azd/pkg/tool/installer.go | Implements uninstall logic for skills + non-skills, adds uninstall command builders, and direct-download artifact removal. |
| cli/azd/pkg/tool/installer_test.go | Extensive installer uninstall test coverage across strategies and skill hosts. |
| cli/azd/cmd/tool.go | Wires up the tool uninstall Cobra/action implementation (flags, host resolution, dry-run, telemetry). |
| cli/azd/cmd/tool_test.go | Command-level tests for uninstall host resolution and telemetry/dry-run behavior. |
| cli/azd/cmd/testdata/TestUsage-azd-tool.snap | Updates azd tool usage snapshot to list uninstall. |
| cli/azd/cmd/testdata/TestUsage-azd-tool-uninstall.snap | New usage snapshot for azd tool uninstall. |
| cli/azd/cmd/testdata/TestFigSpec.ts | Adds Fig completion spec for azd tool uninstall. |
| cli/azd/cmd/telemetry_coverage_test.go | Registers tool uninstall under commands with specific telemetry. |
jongio
left a comment
There was a problem hiding this comment.
The production code is solid and follows existing patterns well. One test has a mock setup bug that causes CI failures on all platforms.
TestUninstall_NonSkill_CommandFails uses &mockDetector{} which defaults to Installed: false. After the npm command fails, runUninstall checks whether the tool is still detected. Since the mock reports it as not installed, the idempotent success path fires (result.Success = true) instead of the intended error path. The test then fails its assert.False(t, result.Success) assertion.
Fix: use installedDetector("1.0.0") so the tool is still detected after the failed command, triggering the packageManagerUninstallFailedError path the test intends to exercise.
…na/azd-tool-uninstall
jongio
left a comment
There was a problem hiding this comment.
Prior feedback addressed: TestUninstall_NonSkill_CommandFails now uses installedDetector("1.0.0"), correctly exercising the failure path.
The skill-before-host uninstall ordering in UninstallTools is a solid design choice with proper test coverage. One minor observation below on dry-run output for skills.
jongio
left a comment
There was a problem hiding this comment.
Prior feedback addressed: TestUninstall_NonSkill_CommandFails now uses installedDetector("1.0.0"), correctly exercising the failure path.
The skill-before-host uninstall ordering in UninstallTools is a solid design choice with proper test coverage. One minor observation below on dry-run output for skills.
RickWinter
left a comment
There was a problem hiding this comment.
This adds azd tool uninstall as the inverse of tool install, threading a new Uninstall through the Installer interface, Manager.UninstallTools, and the cmd action, plus --all, --dry-run, and --host for skills. The shape is right: it mirrors the install/upgrade action structure, reuses the same telemetry emitter, keeps the tool.id vs tool.ids mutual-exclusion discipline, and the idempotent "already gone -> success" handling on every removal path is the correct call. Skill-before-host ordering in UninstallTools (so the host CLI is still on PATH to remove the plugin) is a genuinely good catch and is well covered by tests.
The one thing worth resolving before merge is the duplicated explicit-host resolution loop in installer.go: resolveSkillUninstallTargets copies the existing install-path block verbatim, which is exactly the cross-scope duplication AGENTS.md asks us to hoist. Not blocking, but cheap to fix now while both are fresh.
Nothing here is a security or correctness blocker. Coverage is thorough (manager, installer, and cmd layers all exercised), the docs/telemetry matrices are updated, and the snapshot regeneration is included. Disposition: COMMENT. Address the duplication if you agree; everything else is follow-up at most.
jongio
left a comment
There was a problem hiding this comment.
Reviewed commits dcf0010 and a0d57b8 (the 2 new commits since my last review).
The four extracted helpers preserve the behavior of the inline blocks they replace:
toolIDUsageAttrs: same mutual-exclusion betweentool.id/tool.ids, same sorting for batch, samedry_runpairing. All 4 call sites (install, install dry-run, upgrade, uninstall) pass through identically.explicitSkillHostTargets: samefindSkillHost+ PATH check + error message with supported hosts list. Wired into bothresolveSkillTargets(install/upgrade) andresolveSkillUninstallTargets(uninstall).configuredSkillHostsFor: same installed-to-configured mapping viafindSkillHost. Used in both the upgrade and uninstall default-host paths.findSkillHost: wraps theslices.IndexFunclookup that was duplicated across both callers.
No remaining concerns. My earlier CHANGES_REQUESTED finding (test mock in TestUninstall_NonSkill_CommandFails) was fixed in 842c109. The dry-run host display observation is tracked in #8786.
jongio
left a comment
There was a problem hiding this comment.
Reviewed commits dcf0010 and a0d57b8 (the 2 new commits since my last review).
The four extracted helpers preserve the behavior of the inline blocks they replace:
toolIDUsageAttrs: same mutual-exclusion betweentool.id/tool.ids, same sorting for batch, samedry_runpairing. All 4 call sites (install, install dry-run, upgrade, uninstall) pass through identically.explicitSkillHostTargets: samefindSkillHost+ PATH check + error message with supported hosts list. Wired into bothresolveSkillTargets(install/upgrade) andresolveSkillUninstallTargets(uninstall).configuredSkillHostsFor: same installed-to-configured mapping viafindSkillHost. Used in both the upgrade and uninstall default-host paths.findSkillHost: wraps theslices.IndexFunclookup that was duplicated across both callers.
No remaining concerns. My earlier CHANGES_REQUESTED finding (test mock in TestUninstall_NonSkill_CommandFails) was fixed in 842c109. The dry-run host display observation is tracked in #8786.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Four recurring themes promoted from PR reviews merged since 2026-06-22: 1. go.instructions.md: Test goroutine safety — t.Fatal/require.NoError must not be called from httptest handler goroutines or other non-test goroutines; HTTP headers must be set before WriteHeader. (Source: #8790) 2. extensions.instructions.md: Duplicate helper detection — export shared helpers from the owning package instead of copying function bodies across extension packages. (Source: #8794, #8809) 3. extensions.instructions.md: User-provided path validation — paths from azure.yaml fields (instructions, entryPoint, etc.) must be validated to stay within the expected root directory. (Source: #8779, #8599) 4. extensions.instructions.md: Cross-extension test isolation — tests loading sibling-extension files must skip when absent; extension functional tests should have dedicated CODEOWNERS entries. (Source: #8809, #8754) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #8716
Summary
Adds the missing
azd tool uninstallcommand to theazd toolgroup, completing the install / upgrade / uninstall lifecycle. The command removes installed tools using the same mechanism each tool was installed through, and adds per-host support for skills.Behavior
azd tool uninstall <tool...>— uninstall the named tool(s).--all— uninstall every installed tool.--dry-run— preview what would be uninstalled without making changes.--output json— structured per-tool results.Per-host skills (
--host)For skill tools (e.g.
azure-skills, which install through agent CLIs like copilot/claude):azd tool uninstall <skill>— removes the skill from every host it is installed through.azd tool uninstall <skill> --host copilot— removes it from that host only.azd tool uninstall <skill> --host all— removes it from every detected host.--hosterrors on non-skill tools.Uninstall coverage by tool category
winget uninstall/brew uninstall/apt-get remove/npm uninstall -gcode --uninstall-extension <id>npm uninstall -gplugin uninstall(copilot, claude)azd extension uninstall <id>(via newInstallStrategy.UninstallCommand)After running the uninstall command, success is verified by re-detecting that the tool is no longer present (inverted from install, with the same retry/backoff for non-CLI tools).
Implementation
pkg/tool/installer.go—Uninstallon theInstallerinterface;runUninstall(package-manager / explicitUninstallCommand/ direct-download) andrunSkillUninstall(per-host removal + host-scoped verification);buildUninstallCommand,executeUninstall,executeUninstallCommand,executeDirectDownloadUninstall, and dedicated error builders.pkg/tool/manifest.go— newSkillHost.PluginUninstallCommand(populated for copilot/claude) andInstallStrategy.UninstallCommand(populated for the azd extension).pkg/tool/manager.go—UninstallTools(ctx, ids, opts...), forwarding host options; per-tool failures don't abort the batch; dependencies are intentionally left in place.cmd/tool.go— theuninstallsubcommand,toolUninstallAction/flags,--hostresolution (shared with install/upgrade), and dry-run, reusing the existingrunToolOperationand telemetry plumbing.Telemetry
Reuses the existing
tool.id/tool.ids,tool.dry_run, andtool.install.*aggregate fields (no new fields/events). Updatedtelemetry-coverage_test.go,docs/reference/telemetry-data.md, anddocs/specs/metrics-audit/feature-telemetry-matrix.md.Tests
UninstallCommandpath, still-detected failure, unsupported strategy,buildUninstallCommandtable, and skill per-host removal (default / explicit host / not-installed / unknown host).UninstallToolsdelegation, option forwarding, partial-failure batch, unknown id.TestUsage/TestFigSpecsnapshots.Notes
azd tool uninstallis unavailable for tools with only a custom shellInstallCommandand no reverse command / package manager; those return a clear "remove manually" message.sudo(apt) apply equally to install and uninstall.