Improve extension source and dependency errors - #9361
Conversation
|
Azure Pipelines: 20 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Not ready to approve
Explicit --source misses still take the earlier generic skip path, making the new source-aware branch unreachable.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Improves extension-upgrade errors when registry resolution fails.
Changes:
- Adds registry-aware failure guidance.
- Adds regression coverage for source-less failures.
File summaries
| File | Description |
|---|---|
cli/azd/cmd/extension.go |
Builds source-aware resolution errors. |
cli/azd/cmd/extension_upgrade_test.go |
Verifies the source-less error. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Handle explicit --source misses on the empty FindExtensions path, name the installed source in resolution failures, and add regression coverage for both cases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 419ac517-f2f6-40c9-bdf2-71855270fdf9
Keep the messages focused on what failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 419ac517-f2f6-40c9-bdf2-71855270fdf9
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
🟡 Not ready to approve
Some resolution errors remain misleading or recommend a retry that repeats the same failure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
cli/azd/cmd/extension.go:2227
- This resolution failure still goes through
fail, which printsRetry with: azd extension upgrade <id>. When there is exactly one match from an unrelated source, that retry deterministically reaches this sameResolveUpgradeSource == nilbranch, so the new output gives a misleading recovery action. Direct the user toazd extension listand/or retry with an explicit--sourceinstead.
if res == nil {
return fail(upgradeSourceResolutionError(
extensionId, a.flags.source, installed.Source,
))
cli/azd/cmd/extension.go:2042
- When the installed source is empty (which the resolver treats as
azd) or alreadyazd, this formatssource 'azd' or the main registry, naming the same registry twice. This path is reachable whenever the extension exists only in another configured source, so special-case the main registry before composing the two-source message.
This issue also appears on line 2224 of the same file.
sourceName := installedSource
if sourceName == "" {
sourceName = extensions.MainRegistryName
}
return fmt.Errorf(
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
When the installed source is empty or already azd, say the extension is not available in the main registry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 419ac517-f2f6-40c9-bdf2-71855270fdf9
There was a problem hiding this comment.
🟡 Not ready to approve
Errors remain misleading when --version filters out an otherwise available extension.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
cli/azd/cmd/extension.go:2233
- The resolver also receives matches already filtered by
--version. If the requested version exists only in another registry,res == nilreaches this helper and reports that the extension is unavailable in the stored/main registries, even when other versions are available there. Pass the requested version into the error builder so this failure identifies the unavailable version rather than claiming the extension is absent.
return fail(upgradeSourceResolutionError(
extensionId, a.flags.source, installed.Source,
))
cli/azd/cmd/extension.go:2200
- When
--versionis also set,matchescan be empty because that version is absent even though the extension exists in the requested source. This path then reports that the extension itself was not found, recreating a misleading resolution error. Include the requested version in this error (and add a regression case for--sourceplus an unavailable--version).
This issue also appears on line 2231 of the same file.
if a.flags.source != "" {
return fail(upgradeSourceResolutionError(
extensionId, a.flags.source, installed.Source,
))
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67d5a76d-afca-4ff8-b7cc-b269c5a6ed7d
There was a problem hiding this comment.
🟡 Not ready to approve
The generated retry command uses Go quoting rather than shell-safe argument escaping.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
cli/azd/cmd/extension.go:2078
%qemits Go string syntax, not shell-safe quoting. Valid source names or non-semver version tags can contain shell metacharacters; for example,$()is still evaluated inside the generated POSIX double quotes, while Go's\"quote escaping is not valid PowerShell escaping. A copied retry command can therefore execute substitutions or pass a different value. Render arguments with shell-aware quoting or restrict the accepted values before including them.
command += fmt.Sprintf(" --source %q", source)
}
if version != "" {
command += fmt.Sprintf(" --version %q", version)
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 67d5a76d-afca-4ff8-b7cc-b269c5a6ed7d
There was a problem hiding this comment.
🟡 Not ready to approve
Generated retry commands do not safely quote source names containing spaces or shell metacharacters.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
cli/azd/cmd/extension.go:2075
- [azd-code-reviewer] The retry command inserts the resolved source name verbatim. Source names may contain spaces or shell metacharacters (
validateSourceNamedoes not reject them), so a suggested retry such as--source my registryis parsed as multiple arguments and can execute shell operators. Quote or shell-escape each dynamic argument and add a test for a source name containing spaces/metacharacters.
command += fmt.Sprintf(" --source %s", source)
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
/azp run azure-dev - cli |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
jongio
left a comment
There was a problem hiding this comment.
Four things worth a look, mostly around the new failure paths.
Medium:
upgradeOneExtension:--allcombined with--sourcenow exits non-zero for every installed extension that isn't in that source, where it used to skip them.evaluateDependencyChanges: the case where a satisfying version exists but isn't compatible with the running azd still gets the old generic message and no suggestion.
Low:
displayDependencyUpgradeResults: the suggestion indent is padded for the Skipped prefix but now also runs under Failed.wrapDependencyError: the two dependency errors build their suggestion text in two different places.
Details are inline.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874b9dda-380d-4e72-9591-3d17079ecae7
|
/azp run azure-dev - cli |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
🟡 Not ready to approve
Retry commands need safe argument escaping, and azd compatibility guidance is incorrect for upper-bound constraints.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
cli/azd/pkg/extensions/manager.go:129
RequiredAzdVersionaccepts upper-bound and bounded constraints, so the running azd can be incompatible because it is too new (for example, azd 2.1 with< 2.0.0). Telling the user to upgrade cannot resolve that case. Use direction-neutral guidance that tells the user to switch to an azd version satisfying the constraint.
"Upgrade azd to a version that satisfies %q, then retry.",
cli/azd/cmd/extension.go:2071
- The registered source name is inserted directly into shell guidance, but
validateSourceNameallows shell metacharacters such as;,$, and parentheses. Copying a retry command for such a source can execute unintended shell syntax. Shell-escape every dynamic argument (extensionId,source, andversion) with a cross-platform command-argument formatter, and update the retry-command tests with metacharacter cases.
command += fmt.Sprintf(" --source %s", source)
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874b9dda-380d-4e72-9591-3d17079ecae7
|
/azp run azure-dev - cli |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
🟡 Not ready to approve
Retry commands require shell-safe arguments, and JSON output currently drops the new recovery guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
cli/azd/cmd/extension.go:2151
- [azd-code-reviewer] The new recovery suggestion is lost for
--output json: this branch suppresses console output, whileUpgradeResult.MarshalJSONintentionally omitsSuggestion(pkg/extensions/upgrade_result.go:60-62,69-100), and the returned top-level error is only the aggregate failure count. As a result, dependency upgrade failures do not provide the actionable guidance promised by this change in JSON mode. Add a plain-textsuggestionfield to the JSON result (with coverage) or otherwise surface it in structured output.
baseResult.Suggestion, err = upgradeFailureDetails(err)
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Fixes #7913
Fixes #8944
This PR improves extension diagnostics when upgrade source resolution fails and when an extension dependency exists but does not satisfy the required version constraint.
Upgrade source resolution
--sourceis specified.Dependency resolution
Behavior at a glance
Extension not found in source ''.dependency X required by Y was not founddependency X required by Y was found, but no version satisfies constraint "..."with recovery guidance.Testing
Covered explicit, stored, and main-registry upgrade source failures; unavailable requested versions; combined source/version misses; retry command preservation; missing and version-incompatible dependency classification; and install and upgrade recovery suggestions.