feat(extensions): upgrade and uninstall while an extension is running

[jongio]

Fixes #9307

Summary

azd extension upgrade always failed when the extension had a running process. This makes upgrade and uninstall work by default without stopping anything, and adds --force for when the new version needs to take effect immediately.

Issue

Upgrade uninstalls before it installs, and uninstall deletes the install directory. Windows refuses to delete a file that is mapped as a running executable image, so the removal returned Access is denied. The existing retry loop waits ten seconds, which a process that is genuinely still running will always outlast.

Relocation, so the default path stops failing

Windows allows a locked executable to be renamed even though it refuses to delete it, because the image loader opens executables with FILE_SHARE_DELETE but not FILE_SHARE_WRITE. Files that resist removal are moved into a sibling .trash directory. That empties the install directory, lets the new version be written, and needs no cooperation from the running process.

Relocated files are swept on later extension operations, once the holding process has exited.

--force, so the new version takes effect immediately

Relocation on its own leaves the old process running against the old binary. --force stops the extension's own processes first, and is available on upgrade, uninstall, and install.

Discovery is scoped to executables inside the extension's install directory, so unrelated processes are never candidates. Child processes are deliberately left alone: their executables live elsewhere, which is exactly where that containment guarantee stops holding.

Termination is graceful then forced on Unix. Windows goes straight to termination, since it has no equivalent signal.

Behaviour at a glance

Scenario	Before	After
upgrade while the extension is running	Fails with Access is denied	Succeeds; the old process keeps running the old binary until it exits
uninstall while the extension is running	Fails with Access is denied	Succeeds
upgrade --force while the extension is running	Not available	Stops the extension, then upgrades
Removal still blocked	Bare permission error	Names the blocking processes and their PIDs, points at --force
Nothing running	Unchanged	Unchanged

Hardening

Found by attacking the implementation after it was written, and fixed here:

• Extension ids are validated as a single path component before being joined. This closes a pre-existing traversal gap, and rejects ids that Windows aliases onto a different directory. os.MkdirAll(".trash.") creates .trash, so the reserved-name check was bypassable, and the id foo. resolved onto the directory owned by foo.
• Directories azd owns are refused when a symlink or junction has been planted there, so neither the trash sweep nor the termination scope can be redirected. Both reparse forms are checked: a junction sets ModeIrregular and isn't resolved by filepath.EvalSymlinks, but os.ReadDir and os.Rename still follow it.

Testing

75 new test functions across pkg/processutil, pkg/osutil, pkg/extensions, and cmd. The file-lock and process paths run against real spawned processes and real locked files rather than mocks, since Windows image-lock semantics can't be faked meaningfully. mage preflight passes all nine checks.

Design notes and the full test plan are in docs/specs/extension-upgrade-force/.

[azure-pipelines]

Azure Pipelines:
7 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Enables extension upgrade and uninstall while processes are running, with opt-in forced termination and deferred cleanup.

Changes:

• Adds cross-platform process discovery and termination.
• Relocates locked files into deferred-cleanup trash storage.
• Adds --force, documentation, snapshots, and extensive tests.

Show a summary per file

File	Description
docs/specs/extension-upgrade-force/test-plan.md	Documents test coverage.
docs/specs/extension-upgrade-force/spec.md	Describes the design and security model.
cli/azd/test/proctest/proctest.go	Adds real-process test helpers.
cli/azd/pkg/processutil/processutil.go	Implements scoped discovery and termination.
cli/azd/pkg/processutil/processutil_windows.go	Adds Windows process handling.
cli/azd/pkg/processutil/processutil_windows_test.go	Tests Windows termination behavior.
cli/azd/pkg/processutil/processutil_unix.go	Adds Unix signal handling.
cli/azd/pkg/processutil/processutil_test.go	Tests scoping and validation.
cli/azd/pkg/processutil/processutil_procfs.go	Adds procfs discovery.
cli/azd/pkg/processutil/processutil_procfs_test.go	Tests procfs path normalization.
cli/azd/pkg/processutil/processutil_integration_test.go	Tests real process lifecycle behavior.
cli/azd/pkg/processutil/processutil_darwin.go	Adds macOS process discovery.
cli/azd/pkg/processutil/helper_signals_windows_test.go	Defines Windows helper signals.
cli/azd/pkg/processutil/helper_signals_unix_test.go	Defines Unix helper signals.
cli/azd/pkg/osutil/relocate.go	Implements locked-file relocation and sweeping.
cli/azd/pkg/osutil/relocate_test.go	Tests relocation and link guards.
cli/azd/pkg/extensions/manager.go	Integrates force and trash cleanup into installs/upgrades.
cli/azd/pkg/extensions/manager_uninstall.go	Implements hardened uninstall behavior.
cli/azd/pkg/extensions/manager_uninstall_test.go	Tests uninstall and upgrade integration.
cli/azd/internal/cmd/errors_test.go	Classifies new sentinel errors.
cli/azd/docs/extensions/extension-resolution-and-versioning.md	Documents forced reinstall behavior.
cli/azd/docs/extensions/extension-framework.md	Documents running-extension handling.
cli/azd/cmd/testdata/TestUsage-azd-extension-upgrade.snap	Updates upgrade help snapshot.
cli/azd/cmd/testdata/TestUsage-azd-extension-uninstall.snap	Updates uninstall help snapshot.
cli/azd/cmd/testdata/TestUsage-azd-extension-install.snap	Updates install help snapshot.
cli/azd/cmd/testdata/TestFigSpec.ts	Updates completion metadata.
cli/azd/cmd/extension.go	Wires flags, callbacks, and reporting.
cli/azd/cmd/extension_force_test.go	Tests command-level force behavior.
cli/azd/.vscode/cspell.yaml	Adds Go spelling terms.
.vscode/cspell.misc.yaml	Adds documentation spelling terms.

Review details

Comments suppressed due to low confidence (1)

cli/azd/cmd/extension.go:2383

• [azd-code-reviewer] Stopped processes are retained only in this local slice, while all reporting is gated by !isJsonOutput. Thus azd extension upgrade --force --output json silently omits which processes were terminated, contrary to the linked issue's reporting acceptance criterion. Include stopped-process data in the structured upgrade result and JSON contract, with a JSON-output test.

			OnProcessStopped: func(process processutil.ProcessInfo) {
				stopped = append(stopped, process)
			},

• Files reviewed: 30/30 changed files
• Comments generated: 6
• Review effort level: Medium

[jongio]

Also picking up the low-confidence comment on cmd/extension.go:2383 from the review body, because it was right.

Every reportStoppedProcesses call sits behind !isJsonOutput, so azd extension upgrade --force --output json gave a scripted caller no signal that azd had terminated anything -- which leaves the issue's The command reports which processes it stopped (name and PID) criterion unmet for exactly the automation case it matters most for.

Fixed in 556a90d. UpgradeResult.StoppedProcesses is populated on every exit path that can have stopped something (the failure helper, the already-current dependency-reconcile branch, and the success/promotion path) and serializes as stoppedProcesses with name, pid, and executable. It is omitempty, so runs that stopped nothing produce byte-identical output to before and no existing snapshot changes. Covered by TestUpgradeResult_MarshalJSON (present when populated, absent when not) and asserted end to end in TestExtensionUpgradeAction_ForceReachesStaleDependencyOfCurrentParent.

[Copilot]

Review details

Comments suppressed due to low confidence (1)

cli/azd/pkg/extensions/manager_uninstall.go:281

• [azd-code-reviewer] This second ErrorWithSuggestion also omits Message, causing the raw wrapped filesystem error to be promoted as the main error text. Populate Message so the process-specific explanation is primary and the underlying permission error remains technical detail.

	return &internal.ErrorWithSuggestion{

• Files reviewed: 32/32 changed files
• Comments generated: 4
• Review effort level: Medium

[JeffreyCA]

Thanks for the PR @jongio. The team discussed this today and we think the main issue here is that the current error is too cryptic and doesn't give users a clear indication of what went wrong or how to proceed. During our discussion, we aligned on improving the error message as the highest-priority fix.

Rather than introducing a --force workflow in this change, we'd prefer to keep the scope focused on improving the error experience with a good user suggestion. For example, we could start with something like this as @hyoshis suggested:

Cannot upgrade extension because it is currently in use.
Stop the running extension and try again.

Some other context like extension binary path in use could be useful to include as well.

That would help users understand exactly what needs to be stopped without expanding the scope into new upgrade behaviour.

We can evaluate whether a --force option makes sense separately - happy to also take this to our Teams channel to discuss further.

[jongio]

@JeffreyCA As an end user I disagree with your assessment. I often have a dozen instances of an extension running in watch mode and the only way to close them is to open task manager and manually kill each one individually. This is a poor user experience that can be solved with a --force flag. Can you provide justification for the pushback?

[jongio]

Went back over this after the discussion, against the current head. Build is clean and the touched packages pass locally (go test ./pkg/processutil/... ./pkg/osutil/... ./pkg/extensions/...).

On @JeffreyCA's point about the error message: the code is closer to satisfying it than the thread suggests, but it doesn't actually land it. extensionRemovalError already discovers and names the blocking processes, then discards that by leaving Message empty on ErrorWithSuggestion. What a user sees today is:

ERROR: failed to remove extension, in use by demo.exe (PID 4242): remove C:\Users\me\.azd\extensions\demo\demo.exe: Access is denied.

"Access is denied" is still the headline. Setting Message demotes it to grey technical detail and puts a readable sentence first. That's a few lines and it's the part the team actually asked for. Details inline.

On scope: this bundles three separable changes. The error message fix, the relocation behavior that makes the default path succeed while the extension runs, and the --force termination subsystem. The first is small and nobody disagrees with it. Splitting it out would ship the user visible fix now and let relocation and --force be argued on their own merits instead of blocking each other. The extension id validation in validateExtensionId is a third independent thing: it closes a pre-existing traversal gap that has nothing to do with this feature, and could go in on its own.

On the open bot comment about COM1/COM2/COM3 written with superscript digits: I couldn't reproduce it. On Windows 11 through Go, all four of those plus the LPT variant create as ordinary directories that coexist alongside a separate real COM1, and files written inside them read back correctly. NUL does fail, so the probe is sound. Go goes through the UTF-16 W APIs, so the ANSI codepage folding that comment describes isn't reachable from here. Plain COM1, which is already in the reject list, also creates fine as a directory, so that list is defense in depth against file open semantics rather than a directory aliasing fix. Not adding those six.

[JeffreyCA]

As an end user I disagree with your assessment. I often have a dozen instances of an extension running in watch mode and the only way to close them is to open task manager and manually kill each one individually. This is a poor user experience that can be solved with a --force flag. Can you provide justification for the pushback?

Thanks for the context, definitely see how --force would help for your watch-mode scenario. The main pushback from the team was we prefer to solve the user-facing UX issue first and evaluate a --force flag separately. Today the error is a bit too cryptic and doesn't tell users what's blocking the upgrade or what they should do next.

@richardpark-msft / @kristenwomack / @tg-msft, feel free to weigh in here as well.

[jongio]

As an end user I disagree with your assessment. I often have a dozen instances of an extension running in watch mode and the only way to close them is to open task manager and manually kill each one individually. This is a poor user experience that can be solved with a --force flag. Can you provide justification for the pushback?

Thanks for the context, definitely see how --force would help for your watch-mode scenario. The main pushback from the team was we prefer to solve the user-facing UX issue first and evaluate a --force flag separately. Today the error is a bit too cryptic and doesn't tell users what's blocking the upgrade or what they should do next.

@richardpark-msft / @kristenwomack / @tg-msft, feel free to weigh in here as well.

Are you saying we should split into multiple PRs?

[wbreza]

Automated multi-model code review — COMMENT

Reviewed at 556a90d across three models (runtime/concurrency, security/path-safety, quality/API). This is a carefully engineered and unusually well-documented change: the cross-platform process handling, the rename-into-.trash relocation strategy, and the reparse-point hardening all reflect real attention, and exercising the file-lock and process paths against real spawned processes / real locked files (rather than mocks) is the right call. Most items earlier reviewers raised are already fixed at this SHA.

One Medium concurrency finding is worth addressing before merge (inline). The remaining observations are low-severity and already acknowledged in existing threads.

Medium

• SweepTrash can delete the shared .trash directory out from under a concurrent relocation (cli/azd/pkg/osutil/relocate.go) — see inline comment.

Low / already acknowledged (non-blocking)

• Unix PID-reuse residual window in Terminate (cli/azd/pkg/processutil/processutil.go): the check-then-signal pair remains racy on Unix. This is already documented in the function comment (Windows pins the PID via an open handle; Unix has no portable equivalent) and discussed on the existing thread. Noting for completeness only.
• executableInScope relies on IsPathContained, which rewrites \→/ (processutil.go:304): on Unix an executable whose path contains literal backslashes could be judged out-of-scope and silently skipped by --force. Extremely contrived, and already discussed on the existing thread.

Verified — not a defect

• A "symlink/junction at the scope directory widens the termination scope" concern was raised by one model but does not hold: normalizeScope calls RequireRealDir before filepath.EvalSymlinks, so a link at the scope directory is rejected, not resolved. EvalSymlinks only canonicalizes parent components, as documented.

Scope context (product decision, not a code issue)

• A maintainer review (CHANGES_REQUESTED) asked to keep this focused on improving the error message and to defer the --force workflow, and the author has independently flagged that install --force now also terminates processes (a semantic change to an existing flag). That is a scope decision for the maintainers; this review does not attempt to resolve it.

Generated by an automated reviewer running non-interactively.

[wbreza]

Automated re-review — COMMENT (updated for 0b398ba)

New commit 0b398ba ("address review feedback on extension --force") since my last pass. Re-reviewed the delta (6 files) at the new head, built and ran the affected packages, and reconciled against my prior findings.

Resolved since last review ✅

• executableInScope backslash-rewrite scope gap — fixed. The new host-native pathContained helper (processutil.go) uses filepath.Rel instead of osutil.IsPathContained, so a Unix filename containing literal backslashes is no longer cleaned into scope. The escape check correctly distinguishes a ..cache child (contained) from the exact .. parent (rejected), and filepath.Rel supplies Windows case-folding via sameWord. The scoped change leaves the other seven IsPathContained rejection-gate callers untouched. Covered by TestPathContained and TestFindByExecutableDir_UnixBackslashIsNotASeparator.
• Error-message UX — fixed. extensionRemovalError now sets Message on all three exits, so ux.ErrorWithSuggestion leads with the readable sentence and demotes the filesystem cause to the grey technical block. The new removalMessage/DescribeExecutables name the binary being held open (de-duplicated across PIDs), and the "nothing discovered" branch correctly stops suggesting --force (which only stops processes discovery returned).
• Doc traceability — fixed. spec.md now defines the AC-1..AC-12 acceptance criteria the test plan cited, and the mismatched test-plan rows were retargeted.

Verification: go build + go test ./pkg/processutil/... ./pkg/extensions/... pass at 0b398ba (Windows), go vet clean. New tests are meaningful (dedup, empty, sibling-prefix, exact-parent cases), not tautological.

Still open (carried from prior review)

• Medium — SweepTrash can delete the shared .trash directory mid-relocation (cli/azd/pkg/osutil/relocate.go): unchanged in this delta, so the concurrency race still stands. See inline comment.

Low / acknowledged (non-blocking)

• Unix PID-reuse residual window in Terminate (processutil.go): still present and already documented in the function comment (Windows pins the PID via an open handle; Unix has no portable equivalent). Noting for completeness only.

Generated by an automated reviewer running non-interactively.

[Copilot]

Review details

Comments suppressed due to low confidence (2)

cli/azd/pkg/osutil/relocate.go:138

• [azd-code-reviewer] Removing the shared trash directory races with relocation in another azd process. A sweeper can observe zero entries and remove .trash after the relocator's MkdirAll/RequireRealDir but before os.Rename; that rename then fails because its parent vanished, leaving the locked executable in place and reproducing the upgrade failure. Keep the shared directory once created, or make directory creation and rename resilient to a concurrent sweep, and cover sweep-versus-relocate concurrency (the current test only runs relocations against a pre-created directory).

	if remaining == 0 {
		// Best effort: a concurrent azd process may have written a new entry in between.
		_ = os.Remove(trashDir)

cli/azd/pkg/processutil/processutil.go:227

• [azd-code-reviewer] Cancellation is currently treated like an expired grace period. On Linux and Windows, enumerateProcesses ignores the context, so a canceled --force continues through this re-check and sends the forceful kill; the new cancellation test explicitly permits that outcome. Check ctx.Err() before signaling and again after waitForExit returns so cancellation does not escalate termination.

		if waitForExit(ctx, process.PID, grace) {
			return true, nil
		}
	}

• Files reviewed: 32/32 changed files
• Comments generated: 0 new
• Review effort level: Medium

[wbreza]

Automated re-review — APPROVE (updated for 0fd5fa09)

New commit 0fd5fa09 ("recreate a swept trash directory instead of failing the relocation") since my last pass. This resolves the last open finding.

Resolved ✅ — Medium: SweepTrash .trash race

relocateInto now retries the os.Rename once when the destination has vanished, and the recreate path is factored into ensureTrashDir (MkdirAll + link re-check). The fix is correctly scoped:

• Retry fires only on fs.ErrNotExist — a permission failure or a destination that is itself a locked executable is reported unchanged, not spun on.
• ensureTrashDir repeats RequireRealDir after the recreate, so a link planted in the race window cannot turn the recovery into a redirect — the reparse-point safety property is preserved.
• Tests directly reproduce the swept-directory state (TestRelocateInto_RecreatesSweptTrashDirectory) and assert the platform-split retry guard plus source-left-intact-on-failure (TestRelocateInto_ReportsFailureAndLeavesSourceInPlace, covering Windows ErrNotExist retry vs Unix ENOTDIR short-circuit).

Verification: go build, go test ./pkg/osutil/..., and go vet all pass at 0fd5fa09 (Windows), main working tree untouched.

Prior findings — all resolved

• Backslash IsPathContained scope gap → fixed in 0b398ba (host-native pathContained).
• Error-message Message not set → fixed in 0b398ba.
• Doc traceability (AC-1..AC-12, test-name drift) → fixed in 0b398ba.

Remaining (non-blocking, does not affect approval)

• Low: Unix PID-reuse residual window between the scope re-check and signal in Terminate — inherent to POSIX signalling and already documented in the function comment (Windows pins the PID via an open handle; Unix has no portable equivalent). No change required.

No Critical/High/Medium findings remain. Approving.

Generated by an automated reviewer running non-interactively.

[Copilot]

Review details

Comments suppressed due to low confidence (2)

docs/specs/extension-upgrade-force/spec.md:166

• [azd-code-reviewer] This security description is stale: the implementation explicitly avoids osutil.IsPathContained because its backslash normalization can reinterpret a legal Unix filename and widen the termination scope (pkg/processutil/processutil.go:319-362). Document the host-native filepath.Rel containment now used instead; the current text describes the exact unsafe behavior that the implementation and T77 reject.
  docs/specs/extension-upgrade-force/test-plan.md:49
• [azd-code-reviewer] This row references a test that does not exist; the implemented test is TestFindByExecutableDir_ResolvesSymlinkedAncestors. Update the traceability entry so the claimed automated coverage can be located and run.

• Files reviewed: 32/32 changed files
• Comments generated: 2
• Review effort level: Medium

[tg-msft]

We're rapidly approaching the extension framework GA and trying to lock down major changes. I don't think the risk of big changes to the extension installation experience is worth it right now given the size of the audience this issue impacts. We should look at improving this in the future post-GA.

For now I think improving the error message (including dumping out the pid if possible so it's trivial to kill yourself) would be a reasonable stopgap without risk of a bug trail.

[jongio]

We're rapidly approaching the extension framework GA and trying to lock down major changes. I don't think the risk of big changes to the extension installation experience is worth it right now given the size of the audience this issue impacts. We should look at improving this in the future post-GA.

For now I think improving the error message (including dumping out the pid if possible so it's trivial to kill yourself) would be a reasonable stopgap without risk of a bug trail.

Understandable, thanks for the context. I'll close this PR and keep the issue open.