Honor installed extension source during upgrade#6720
Conversation
- Modified extensionUpgradeAction to use installed extension's source when --source flag not provided - Added checkForNewerVersionInOtherSources to warn users if newer version exists in different source - Updated error message to include source name when extension not found - Added test placeholder for source honoring behavior Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
- Replace log.Printf with console.MessageUxItem for consistent logging - Remove unused log import - Remove placeholder test that provided no value Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
- Remove unnecessary selectDistinctExtension call since filtering by exact source guarantees single match - Simplify version comparison logic by tracking newest semver directly instead of nested conditionals Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
- Add defensive check for multiple matches when filtering by source and ID - Improve warning message clarity to explain that upgrade will proceed with original source version - Change "upgrade to" to "switch to" in instructions for clarity Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
The HidePrefix field defaults to false, so explicitly setting it is unnecessary Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
- Extract warning description to variable for better readability - Extract instruction message to variable - Extract command strings to variables before formatting - Make code more maintainable and easier to read Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses an issue where azd extension upgrade --all would prompt users to select a source when an extension exists in multiple sources, even though the extension was already installed from a specific source. The fix ensures that the upgrade operation honors the source from which the extension was originally installed, and provides warnings when newer versions are available in different sources.
Changes:
- Modified upgrade logic to use the installed extension's source by default, while still allowing users to override via
--sourceflag - Added cross-source version checking to warn users when newer versions are available in different sources
- Improved error messages to include source information
- Promoted jmespath dependency from indirect to direct (cleanup)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| cli/azd/go.mod | Moved jmespath-community/go-jmespath from indirect to direct dependencies |
| cli/azd/cmd/extensions_test.go | Added trailing newline (formatting) |
| cli/azd/cmd/extension.go | Implemented source-aware upgrade logic and cross-source version checking with warnings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // When filtering by exact source and extension ID, there should only be one match | ||
| // However, if there are multiple matches (unexpected), we'll use the first one | ||
| if len(matches) > 1 { | ||
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | ||
| Description: fmt.Sprintf( | ||
| "Found %d matches for %s in source %s, using the first match", | ||
| len(matches), | ||
| extensionId, | ||
| sourceToUse, | ||
| ), | ||
| }) |
There was a problem hiding this comment.
The warning for multiple matches (lines 942-951) indicates an unexpected state when filtering by exact source and extension ID. However, this warning uses the first match without any explanation of why the first one was chosen or what criteria differentiate the matches. Consider either: (1) sorting the matches by version (descending) and documenting this in the warning, or (2) returning an error since this truly is an unexpected/invalid state in the extension registry that should be investigated rather than silently worked around.
| // When filtering by exact source and extension ID, there should only be one match | |
| // However, if there are multiple matches (unexpected), we'll use the first one | |
| if len(matches) > 1 { | |
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | |
| Description: fmt.Sprintf( | |
| "Found %d matches for %s in source %s, using the first match", | |
| len(matches), | |
| extensionId, | |
| sourceToUse, | |
| ), | |
| }) | |
| // When filtering by exact source and extension ID, there should only be one match. | |
| // If there are multiple matches, this indicates an invalid or inconsistent state in the | |
| // extension registry; fail explicitly so this can be investigated rather than silently | |
| // proceeding with an arbitrary selection. | |
| if len(matches) > 1 { | |
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | |
| return nil, fmt.Errorf( | |
| "found %d matches for extension %s in source %s; expected exactly one match", | |
| len(matches), | |
| extensionId, | |
| sourceToUse, | |
| ) |
| // Honor the source from which the extension was originally installed | ||
| // Only use the --source flag if explicitly provided by the user | ||
| sourceToUse := installed.Source | ||
| if a.flags.source != "" { | ||
| sourceToUse = a.flags.source | ||
| } |
There was a problem hiding this comment.
The logic for honoring the installed extension's source is good, but there's a potential edge case when --all is used with --source. If a user runs azd extension upgrade --all --source specific-source, all installed extensions will be attempted to be upgraded from specific-source, even if they were originally installed from different sources. This will fail with "extension X not found in source specific-source" for extensions not available in that source. Consider adding validation to prevent this confusing combination, similar to the existing check for --version with multiple extensions at line 870-872.
| // Honor the source from which the extension was originally installed | ||
| // Only use the --source flag if explicitly provided by the user | ||
| sourceToUse := installed.Source | ||
| if a.flags.source != "" { | ||
| sourceToUse = a.flags.source | ||
| } | ||
|
|
||
| filterOptions := &extensions.FilterOptions{ | ||
| Id: extensionId, | ||
| Source: a.flags.source, | ||
| Source: sourceToUse, | ||
| Version: a.flags.version, | ||
| } | ||
|
|
||
| matches, err := a.extensionManager.FindExtensions(ctx, filterOptions) | ||
| if err != nil { | ||
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | ||
| return nil, fmt.Errorf("failed to get extension %s: %w", extensionId, err) | ||
| } | ||
|
|
||
| if len(matches) == 0 { | ||
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | ||
| return nil, fmt.Errorf("extension %s not found", extensionId) | ||
| return nil, fmt.Errorf("extension %s not found in source %s", extensionId, sourceToUse) | ||
| } | ||
|
|
||
| selectedExtension, err := selectDistinctExtension(ctx, a.console, extensionId, matches, a.flags.global) | ||
| if err != nil { | ||
| return nil, err | ||
| // When filtering by exact source and extension ID, there should only be one match | ||
| // However, if there are multiple matches (unexpected), we'll use the first one | ||
| if len(matches) > 1 { | ||
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | ||
| Description: fmt.Sprintf( | ||
| "Found %d matches for %s in source %s, using the first match", | ||
| len(matches), | ||
| extensionId, | ||
| sourceToUse, | ||
| ), | ||
| }) | ||
| } | ||
| selectedExtension := matches[0] | ||
|
|
||
| a.console.ShowSpinner(ctx, stepMessage, input.Step) | ||
| latestVersion := selectedExtension.Versions[len(selectedExtension.Versions)-1] | ||
|
|
||
| // Check if there's a newer version available in other sources | ||
| // Only do this if the user didn't explicitly specify a source | ||
| if a.flags.source == "" { | ||
| if err := a.checkForNewerVersionInOtherSources( | ||
| ctx, | ||
| extensionId, | ||
| installed.Source, | ||
| latestVersion.Version, | ||
| ); err != nil { | ||
| // Log the error but don't fail the upgrade | ||
| // Using console warning instead of log.Printf for consistency | ||
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | ||
| Description: fmt.Sprintf( | ||
| "Failed to check for newer versions in other sources: %v", | ||
| err, | ||
| ), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Parse semantic versions for proper comparison | ||
| installedSemver, err := semver.NewVersion(installed.Version) | ||
| if err != nil { |
There was a problem hiding this comment.
The new behavior of honoring the installed extension's source and checking for newer versions in other sources lacks test coverage. Given that the codebase has comprehensive test infrastructure (as seen in extensions_test.go, manager tests, etc.), this significant behavioral change should include tests covering:
- Upgrading with default behavior (honoring original source)
- Upgrading with explicit --source flag (overriding original source)
- Warning when newer version exists in different source
- Handling of multiple matches in same source
- Error handling when extension not found in original source
| // Check if there's a newer version available in other sources | ||
| // Only do this if the user didn't explicitly specify a source | ||
| if a.flags.source == "" { | ||
| if err := a.checkForNewerVersionInOtherSources( | ||
| ctx, | ||
| extensionId, | ||
| installed.Source, | ||
| latestVersion.Version, | ||
| ); err != nil { | ||
| // Log the error but don't fail the upgrade | ||
| // Using console warning instead of log.Printf for consistency | ||
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | ||
| Description: fmt.Sprintf( | ||
| "Failed to check for newer versions in other sources: %v", | ||
| err, | ||
| ), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
The warning about newer versions in other sources is displayed while the spinner is still active (line 954 shows spinner, lines 957-975 show the warning). This will result in the warning being interleaved with the spinner output, making it hard to read. The warning should be displayed either before the spinner starts or after it's stopped. Consider moving the checkForNewerVersionInOtherSources call to after line 1011 (after the spinner is stopped) or stopping the spinner before showing the warning and restarting it afterward.
| } | ||
|
|
||
| if len(matches) == 0 { | ||
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) |
There was a problem hiding this comment.
The error message when an extension is not found in the expected source now includes the source name, which is an improvement. However, when this error occurs during --all upgrades, it will cause the entire upgrade operation to fail and stop processing remaining extensions. Consider whether it would be better to skip the failed extension with a warning and continue with the remaining extensions, similar to how the newer version check errors are handled (lines 966-974).
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | |
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | |
| // When upgrading all extensions, skip missing extensions with a warning | |
| // instead of failing the entire operation. | |
| if a.flags.all { | |
| a.console.MessageUxItem(ctx, &ux.WarningMessage{ | |
| Description: fmt.Sprintf( | |
| "Skipping upgrade for extension %s because it was not found in source %s", | |
| extensionId, | |
| sourceToUse, | |
| ), | |
| }) | |
| return nil, nil | |
| } |
| instructionMsg := fmt.Sprintf( | ||
| "To switch to the newer version from '%s', first uninstall the extension and then "+ | ||
| "install it from the desired source:", | ||
| newerVersionSource, | ||
| ) | ||
| a.console.Message(ctx, instructionMsg) | ||
|
|
||
| uninstallCmd := fmt.Sprintf("azd extension uninstall %s", extensionId) | ||
| a.console.Message(ctx, fmt.Sprintf(" %s", output.WithHighLightFormat(uninstallCmd))) | ||
|
|
||
| installCmd := fmt.Sprintf("azd extension install %s --source %s", extensionId, newerVersionSource) | ||
| a.console.Message(ctx, fmt.Sprintf(" %s", output.WithHighLightFormat(installCmd))) | ||
|
|
||
| a.console.Message(ctx, "") | ||
| } |
There was a problem hiding this comment.
The instruction message for switching sources suggests running two separate commands (uninstall + install). However, this workflow could lose user configuration or settings associated with the extension. Consider adding a note about this potential data loss, or alternatively, document whether the extension manager preserves configuration across uninstall/install operations. Additionally, it might be helpful to mention that the user can specify --source <source> flag directly with the upgrade command to override the default behavior.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
| if newestSemver != nil { | ||
| warningDesc := fmt.Sprintf( | ||
| "A newer version (%s) of %s is available in source '%s', but the extension was installed "+ | ||
| "from source '%s'. The extension will be upgraded to version %s from the original source.", | ||
| output.WithHighLightFormat(newestSemver.String()), | ||
| output.WithHighLightFormat(extensionId), | ||
| output.WithHighLightFormat(newerVersionSource), | ||
| output.WithHighLightFormat(currentSource), | ||
| output.WithHighLightFormat(currentLatestVersion), | ||
| ) | ||
| a.console.MessageUxItem(ctx, &ux.WarningMessage{Description: warningDesc}) |
There was a problem hiding this comment.
What if we showed the message after the upgrade instead of before and made it a little more concise? It feels a little verbose currently. Maybe something like:
E.g. upgrading from 0.0.1 to 0.0.2:
(✓) Done: Upgrading microsoft.azd.concurx extension (0.0.2)
A newer version 0.1.0 is available in source 'azd' (installed: 0.0.2 from 'local').
To switch: azd ext uninstall microsoft.azd.concurx && azd ext install microsoft.azd.concurx --source azd
If no upgrade available on current source:
(-) Skipped: Upgrading microsoft.azd.concurx extension (No upgrade available)
A newer version 0.1.0 is available in source 'azd' (installed: 0.0.2 from 'local').
To switch: azd ext uninstall microsoft.azd.concurx && azd ext install microsoft.azd.concurx --source azd
| matches, err := a.extensionManager.FindExtensions(ctx, filterOptions) | ||
| if err != nil { | ||
| a.console.StopSpinner(ctx, stepMessage, input.StepFailed) | ||
| return nil, fmt.Errorf("failed to get extension %s: %w", extensionId, err) | ||
| } |
There was a problem hiding this comment.
I was testing this and there seems to be a bug with how FindExtensions caches extension data with filters, causing azd ext upgrade --all to not display the warning but azd ext upgrade <id> works. Copilot summary:
Problem
getSources() incorrectly caches filtered results, causing subsequent calls with different filters to return stale/incomplete sources.
Root Cause
getSources() caches sources in memory after the first call. The bug is that it caches the filtered result instead of all sources:
func (tm *Manager) getSources(ctx context.Context, filter sourceFilterPredicate) ([]Source, error) {
if tm.sources != nil {
return tm.sources, nil // Returns cached sources (but these may be filtered!)
}
// ...
sources, err := tm.createSourcesFromConfig(ctx, configs, filter) // Creates with filter applied
tm.sources = sources // Caches the FILTERED result
return tm.sources, nil
}Example Scenario
- Code calls
FindExtensions(Source: "local")- caches only "local" source - Later code calls
FindExtensions(Id: "some.extension")with no source filter - Despite no filter, only "local" source is searched (cached result)
- Extensions from "azd" or other sources are never found
The order of calls determines what gets cached, making behavior unpredictable and dependent on execution order.
azd extension upgrade --allprompts for source selection when an extension exists in multiple sources, even though it's already installed from a specific source.Changes
extensionUpgradeAction.Run(): Use installed extension's source when--sourceflag not providedinstalled.Sourcebefore version lookup--sourceflagcheckForNewerVersionInOtherSources(): New helper to detect cross-source updatesBehavior
Before:
After:
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
aka.ms/tmp/azd /tmp/azd version /usr/bin/head head -n ration. All rights reserved. cli/azd/pkg/tools/kubectl/kube_config.go /usr/bin/tail ration. All righbash cli/azd/cmd/midd--norc /usr/bin/tail tail -n e. tail /usr/bin/grep e. o /usr/bin/grep grep(dns block)/tmp/azd /tmp/azd extension source list -q r the MIT License. o /usr/bin/grep r the MIT Licensbash ecker_test.go /usr/bin/grep grep -q // Licensed under the MIT License. ut/formatter.go /usr/bin/head // Licensed undegit ronment/target_radd /usr/bin/head head(dns block)/tmp/azd /tmp/azd telemetry upload head -n ts reserved. si/armmsi.go /usr/bin/head ts reserved. ost/aca_ingress.--norc /usr/bin/head head -n ration. All rights reserved. cli/azd/pkg/tools/javac/javac.go /usr/bin/tail ration. All righgit cli/azd/cmd/autoadd /usr/bin/tail tail(dns block)westus-0.in.applicationinsights.azure.com/tmp/azd /tmp/azd telemetry upload head -n ts reserved. si/armmsi.go /usr/bin/head ts reserved. ost/aca_ingress.--norc /usr/bin/head head -n ration. All rights reserved. cli/azd/pkg/tools/javac/javac.go /usr/bin/tail ration. All righgit cli/azd/cmd/autoadd /usr/bin/tail tail(dns block)/tmp/azd /tmp/azd telemetry upload grep -q r the MIT License. grep /usr/bin/grep r the MIT Licensbash s_test.go /usr/bin/grep grep -q // Licensed under the MIT License. _manager_test.go /usr/bin/head // Licensed undegit ersion_linux.go /usr/bin/head head(dns block)/tmp/azd /tmp/azd telemetry upload grep -q // Licensed under the MIT License. go /usr/bin/head // Licensed undego test.go /usr/bin/head head -n ts reserved. ut/ux/show.go /usr/bin/head ts reserved. h/watch.go /usr/bin/head head(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.