From 1df562be5068b47315846679b11fdd3412fc7d61 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 5 May 2026 17:22:16 +0000 Subject: [PATCH 01/10] wip --- .../provisioning/bicep/bicep_provider.go | 49 ++++++-- .../provisioning/bicep/local_preflight.go | 13 ++ cli/azd/pkg/output/ux/preflight_report.go | 80 ++++++++++++- .../pkg/output/ux/preflight_report_test.go | 112 ++++++++++++++++++ 4 files changed, 240 insertions(+), 14 deletions(-) diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index f9a309a10f8..cc20071f2db 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -2636,10 +2636,19 @@ func (p *BicepProvider) validatePreflight( if len(results) > 0 { report := &ux.PreflightReport{} for _, result := range results { + var links []ux.PreflightReportLink + for _, l := range result.Links { + links = append(links, ux.PreflightReportLink{ + URL: l.URL, + Title: l.Title, + }) + } report.Items = append(report.Items, ux.PreflightReportItem{ IsError: result.Severity == PreflightCheckError, DiagnosticID: result.DiagnosticID, Message: result.Message, + Suggestion: result.Suggestion, + Links: links, }) } p.console.MessageUxItem(ctx, report) @@ -2656,8 +2665,7 @@ func (p *BicepProvider) validatePreflight( if report.HasWarnings() { p.console.Message(ctx, "") continueDeployment, promptErr := p.console.Confirm(ctx, input.ConsoleOptions{ - Message: "Preflight validation found warnings that may cause the " + - "deployment to fail. Do you want to continue?", + Message: "Proceed with deployment despite the warnings above?", DefaultValue: true, }) if promptErr != nil { @@ -2925,15 +2933,19 @@ func (p *BicepProvider) checkAiModelQuota( DiagnosticID: "ai_model_not_found", Message: fmt.Sprintf( "model %s%s was not found in the AI model "+ - "catalog for %s. The deployment may fail "+ - "if this model is not available. Verify "+ - "the model name, SKU, and version are "+ - "correct. See %s for supported models and regions.", + "catalog for %s.", output.WithHighLightFormat(fmt.Sprintf("%q", dep.ModelName)), output.WithGrayFormat(details), output.WithHighLightFormat(loc), - output.WithLinkFormat("https://learn.microsoft.com/azure/ai-services/openai/concepts/models"), ), + Suggestion: "Verify the model name, SKU, and version are correct. " + + "The deployment will likely fail if this model is not available in this region.", + Links: []PreflightCheckLink{ + { + URL: "https://learn.microsoft.com/azure/ai-services/openai/concepts/models", + Title: "Azure OpenAI supported models and regions", + }, + }, }) continue } @@ -2968,20 +2980,35 @@ func (p *BicepProvider) checkAiModelQuota( totalRequired := requiredByUsage[r.usageName] if remaining < totalRequired { reportedUsage[r.usageName] = true + + suggestion := fmt.Sprintf( + "Reduce the requested capacity to %.0f"+ + " or change your deployment location via %s."+ + " You can also request a quota increase in the Azure portal.", + remaining, + output.WithHighLightFormat( + "azd env set AZURE_LOCATION "), + ) + results = append(results, PreflightCheckResult{ Severity: PreflightCheckWarning, DiagnosticID: "ai_model_quota_exceeded", Message: fmt.Sprintf( - "insufficient quota for model %s %s in %s. "+ - "Requested capacity: %.0f, remaining quota: %.0f. "+ - "The deployment may fail. Consider reducing capacity, "+ - "selecting a different model, or requesting a quota increase.", + "insufficient quota for model %s %s in %s."+ + " Requested capacity: %.0f, remaining quota: %.0f.", output.WithHighLightFormat("%q", r.dep.ModelName), output.WithGrayFormat("(SKU: %s)", r.dep.SkuName), output.WithHighLightFormat(loc), totalRequired, remaining, ), + Suggestion: suggestion, + Links: []PreflightCheckLink{ + { + URL: "https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal", + Title: "Increase Azure subscription quotas", + }, + }, }) } } diff --git a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go index 41d397e30ac..9f4471e98fe 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go +++ b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go @@ -271,6 +271,19 @@ type PreflightCheckResult struct { DiagnosticID string // Message is a human-readable description of the finding. Message string + // Suggestion is an optional actionable recommendation for resolving the issue. + // It should be dynamically generated with context-specific advice when possible. + Suggestion string + // Links is an optional list of reference links related to the finding. + Links []PreflightCheckLink +} + +// PreflightCheckLink represents a reference link attached to a preflight check result. +type PreflightCheckLink struct { + // URL is the link target. + URL string + // Title is the display text (optional — if empty, the URL is shown). + Title string } // validationContext provides the data and utilities available to preflight check functions. diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index 7ac957da34d..2ac2d3200f3 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -20,6 +20,18 @@ type PreflightReportItem struct { DiagnosticID string // Message describes the finding. Message string + // Suggestion is an optional actionable recommendation for resolving the issue. + Suggestion string + // Links is an optional list of reference links related to the finding. + Links []PreflightReportLink +} + +// PreflightReportLink represents a reference link attached to a preflight report item. +type PreflightReportLink struct { + // URL is the link target. + URL string + // Title is the display text (optional — if empty, the URL is shown). + Title string } // PreflightReport displays the results of local preflight validation. @@ -41,6 +53,7 @@ func (r *PreflightReport) ToString(currentIndentation string) string { sb.WriteString("\n") } sb.WriteString(fmt.Sprintf("%s%s %s", currentIndentation, warningPrefix, w.Message)) + writeItemSuggestion(&sb, currentIndentation, w) } if len(warnings) > 0 && len(errors) > 0 { @@ -52,17 +65,78 @@ func (r *PreflightReport) ToString(currentIndentation string) string { sb.WriteString("\n") } sb.WriteString(fmt.Sprintf("%s%s %s", currentIndentation, failedPrefix, e.Message)) + writeItemSuggestion(&sb, currentIndentation, e) } return sb.String() } +// writeItemSuggestion appends the suggestion and links for a report item, if present. +func writeItemSuggestion(sb *strings.Builder, indent string, item PreflightReportItem) { + if item.Suggestion != "" { + sb.WriteString(fmt.Sprintf("\n%s %s %s", + indent, + output.WithHighLightFormat("Suggestion:"), + item.Suggestion)) + } + for _, link := range item.Links { + if link.Title != "" { + sb.WriteString(fmt.Sprintf("\n%s • %s", + indent, + output.WithHyperlink(link.URL, link.Title))) + } else { + sb.WriteString(fmt.Sprintf("\n%s • %s", + indent, + output.WithLinkFormat(link.URL))) + } + } +} + func (r *PreflightReport) MarshalJSON() ([]byte, error) { warnings, errors := r.partition() - return json.Marshal(output.EventForMessage( - fmt.Sprintf("preflight: %d warning(s), %d error(s)", - len(warnings), len(errors)))) + type jsonLink struct { + URL string `json:"url"` + Title string `json:"title,omitempty"` + } + type jsonItem struct { + Severity string `json:"severity"` + DiagnosticID string `json:"diagnosticId,omitempty"` + Message string `json:"message"` + Suggestion string `json:"suggestion,omitempty"` + Links []jsonLink `json:"links,omitempty"` + } + + items := make([]jsonItem, 0, len(r.Items)) + for _, item := range r.Items { + severity := "warning" + if item.IsError { + severity = "error" + } + ji := jsonItem{ + Severity: severity, + DiagnosticID: item.DiagnosticID, + Message: item.Message, + Suggestion: item.Suggestion, + } + for _, link := range item.Links { + ji.Links = append(ji.Links, jsonLink(link)) + } + items = append(items, ji) + } + + result := struct { + Type string `json:"type"` + Summary string `json:"summary"` + Items []jsonItem `json:"items"` + }{ + Type: "preflight", + Summary: fmt.Sprintf("preflight: %d warning(s), %d error(s)", + len(warnings), len(errors)), + Items: items, + } + + return json.Marshal(result) } // HasErrors returns true if the report contains at least one error-level item. diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index c74dbd7c535..a3d489149a3 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -81,6 +81,118 @@ func TestPreflightReport_MarshalJSON(t *testing.T) { require.Contains(t, string(data), "1 error(s)") } +func TestPreflightReport_WarningWithSuggestion(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + Message: "insufficient quota for model gpt-4o", + Suggestion: "Reduce capacity to 140 or change location.", + }, + }, + } + + result := report.ToString(" ") + require.Contains(t, result, "insufficient quota for model gpt-4o") + require.Contains(t, result, "Suggestion:") + require.Contains(t, result, "Reduce capacity to 140 or change location.") + + // Suggestion should appear after the warning message + warnIdx := indexOf(result, "insufficient quota") + suggIdx := indexOf(result, "Reduce capacity") + require.Greater(t, suggIdx, warnIdx, "suggestion should appear after warning message") +} + +func TestPreflightReport_WarningWithLinks(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + Message: "model not found", + Suggestion: "Verify the model name.", + Links: []PreflightReportLink{ + {URL: "https://example.com/models", Title: "Supported models"}, + {URL: "https://example.com/raw-link"}, + }, + }, + }, + } + + result := report.ToString(" ") + require.Contains(t, result, "model not found") + require.Contains(t, result, "Suggestion:") + require.Contains(t, result, "Verify the model name.") + // In non-terminal mode, WithHyperlink falls back to plain URL + require.Contains(t, result, "https://example.com/models") + require.Contains(t, result, "https://example.com/raw-link") +} + +func TestPreflightReport_NoSuggestion(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + {IsError: false, Message: "simple warning"}, + }, + } + + result := report.ToString("") + require.Contains(t, result, "simple warning") + require.NotContains(t, result, "Suggestion:") +} + +func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "ai_model_quota_exceeded", + Message: "insufficient quota", + Suggestion: "Reduce capacity to 140.", + Links: []PreflightReportLink{ + {URL: "https://example.com/quotas", Title: "Quota docs"}, + }, + }, + {IsError: true, DiagnosticID: "role_error", Message: "role missing"}, + }, + } + + data, err := json.Marshal(report) + require.NoError(t, err) + + var parsed struct { + Type string `json:"type"` + Summary string `json:"summary"` + Items []struct { + Severity string `json:"severity"` + DiagnosticID string `json:"diagnosticId"` + Message string `json:"message"` + Suggestion string `json:"suggestion"` + Links []struct { + URL string `json:"url"` + Title string `json:"title"` + } `json:"links"` + } `json:"items"` + } + require.NoError(t, json.Unmarshal(data, &parsed)) + + require.Equal(t, "preflight", parsed.Type) + require.Contains(t, parsed.Summary, "1 warning(s)") + require.Contains(t, parsed.Summary, "1 error(s)") + require.Len(t, parsed.Items, 2) + + // First item: warning with suggestion and links + require.Equal(t, "warning", parsed.Items[0].Severity) + require.Equal(t, "ai_model_quota_exceeded", parsed.Items[0].DiagnosticID) + require.Equal(t, "Reduce capacity to 140.", parsed.Items[0].Suggestion) + require.Len(t, parsed.Items[0].Links, 1) + require.Equal(t, "https://example.com/quotas", parsed.Items[0].Links[0].URL) + require.Equal(t, "Quota docs", parsed.Items[0].Links[0].Title) + + // Second item: error without suggestion + require.Equal(t, "error", parsed.Items[1].Severity) + require.Empty(t, parsed.Items[1].Suggestion) + require.Empty(t, parsed.Items[1].Links) +} + func TestPreflightReport_Indentation(t *testing.T) { report := &PreflightReport{ Items: []PreflightReportItem{ From 3397161d153443468f35845b5bf6278d29137664 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 5 May 2026 22:07:58 +0000 Subject: [PATCH 02/10] Add actionable suggestions and multi-line formatting to preflight warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend preflight warnings with optional Suggestion and Links fields so that each warning can provide context-specific, actionable next steps. This is the first step of the validation improvement roadmap (actionable suggestions → targeted fixes → agentic fixes). Changes: - Add Suggestion/Links fields to PreflightCheckResult and PreflightReportItem - Render suggestions and links below each warning in ToString() - Include structured suggestion data in MarshalJSON() for machine consumption - Generate dynamic suggestions for AI quota warnings: - ai_model_not_found: verify model/SKU/version + docs link - ai_model_quota_exceeded: reduce capacity or change location + portal link - Add suggestions to role_assignment and reserved_resource_name warnings - Reformat all warnings to two-line layout (title + detail) with proper indentation on continuation lines - Update confirm prompt to be more concise - Add unit tests for suggestion rendering, multi-line indentation, and JSON Fixes #8058 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../provisioning/bicep/bicep_provider.go | 96 ++++++++++++------- cli/azd/pkg/output/ux/preflight_report.go | 26 +++-- .../pkg/output/ux/preflight_report_test.go | 47 +++++++++ .../test/functional/preflight_quota_test.go | 10 +- 4 files changed, 133 insertions(+), 46 deletions(-) diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index cc20071f2db..db00dc2f609 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -2763,16 +2763,25 @@ func (p *BicepProvider) checkRoleAssignmentPermissions( Severity: PreflightCheckWarning, DiagnosticID: "role_assignment_missing", Message: fmt.Sprintf( - "the current principal %s does not have permission to create role assignments "+ - "%s on subscription %s. "+ - "The deployment includes role assignments and will fail without this permission. "+ - "Ensure you have the 'Role Based Access Control Administrator', "+ - "'User Access Administrator', 'Owner', or a custom role with "+ - "'Microsoft.Authorization/roleAssignments/write' assigned to your account.", - output.WithHighLightFormat("(%s)", principalId), - output.WithGrayFormat("(Microsoft.Authorization/roleAssignments/write)"), + "Principal %s lacks role assignment"+ + " permissions on subscription %s\n"+ + "The deployment includes role assignments"+ + " and will fail without %s permission.", + output.WithHighLightFormat( + "(%s)", principalId), output.WithHighLightFormat(subscriptionId), + output.WithGrayFormat( + "Microsoft.Authorization/"+ + "roleAssignments/write"), ), + Suggestion: "Ensure you have the" + + " 'Role Based Access Control" + + " Administrator'," + + " 'User Access Administrator'," + + " 'Owner', or a custom role with" + + " 'Microsoft.Authorization/" + + "roleAssignments/write' assigned" + + " to your account.", }}, nil } @@ -2781,14 +2790,17 @@ func (p *BicepProvider) checkRoleAssignmentPermissions( Severity: PreflightCheckWarning, DiagnosticID: "role_assignment_conditional", Message: fmt.Sprintf( - "the current principal %s has conditional permission to create role "+ - "assignments %s on "+ - "subscription %s. The role assignment that grants this permission "+ - "has an ABAC condition that may restrict which roles can be assigned. "+ - "The deployment may fail if the condition does not permit the "+ - "specific role assignments in the template.", - output.WithHighLightFormat("(%s)", principalId), - output.WithGrayFormat("(Microsoft.Authorization/roleAssignments/write)"), + "Principal %s has conditional role"+ + " assignment permissions on"+ + " subscription %s\n"+ + "An ABAC condition may restrict"+ + " which roles can be assigned."+ + " The deployment may fail if the"+ + " condition does not permit the"+ + " specific role assignments in"+ + " the template.", + output.WithHighLightFormat( + "(%s)", principalId), output.WithHighLightFormat(subscriptionId), ), }}, nil @@ -2813,17 +2825,29 @@ func (p *BicepProvider) checkReservedResourceNames( continue } for _, v := range findReservedResourceNameViolations(resource.Name) { - resourceName := output.WithHighLightFormat("%q", resource.Name) - resourceType := output.WithGrayFormat("(%s)", resource.Type) - link := output.WithLinkFormat(docsLink) + resourceName := output.WithHighLightFormat( + "%q", resource.Name) + resourceType := output.WithGrayFormat( + "(%s)", resource.Type) results = append(results, PreflightCheckResult{ Severity: PreflightCheckWarning, DiagnosticID: "reserved_resource_name", Message: fmt.Sprintf( - "resource %s %s %s the reserved word %q. See %s.", - resourceName, resourceType, v.matchType, v.reservedWord, link, + "Resource %s %s %s the"+ + " reserved word %q\n"+ + "Azure does not allow reserved"+ + " words in resource names."+ + " The deployment will fail.", + resourceName, resourceType, + v.matchType, v.reservedWord, ), + Links: []PreflightCheckLink{ + { + URL: docsLink, + Title: "Reserved resource name errors", + }, + }, }) } } @@ -2932,18 +2956,23 @@ func (p *BicepProvider) checkAiModelQuota( Severity: PreflightCheckWarning, DiagnosticID: "ai_model_not_found", Message: fmt.Sprintf( - "model %s%s was not found in the AI model "+ - "catalog for %s.", - output.WithHighLightFormat(fmt.Sprintf("%q", dep.ModelName)), + "Model %s%s not found in %s\n"+ + "Model not found in AI model catalog."+ + " Provisioning will likely fail.", + output.WithHighLightFormat( + "%q", dep.ModelName), output.WithGrayFormat(details), output.WithHighLightFormat(loc), ), - Suggestion: "Verify the model name, SKU, and version are correct. " + - "The deployment will likely fail if this model is not available in this region.", + Suggestion: "Verify the model name, SKU," + + " and version are correct.", Links: []PreflightCheckLink{ { - URL: "https://learn.microsoft.com/azure/ai-services/openai/concepts/models", - Title: "Azure OpenAI supported models and regions", + URL: "https://learn.microsoft.com/" + + "azure/ai-services/openai/" + + "concepts/models", + Title: "Azure OpenAI supported" + + " models and regions", }, }, }) @@ -2994,10 +3023,13 @@ func (p *BicepProvider) checkAiModelQuota( Severity: PreflightCheckWarning, DiagnosticID: "ai_model_quota_exceeded", Message: fmt.Sprintf( - "insufficient quota for model %s %s in %s."+ - " Requested capacity: %.0f, remaining quota: %.0f.", - output.WithHighLightFormat("%q", r.dep.ModelName), - output.WithGrayFormat("(SKU: %s)", r.dep.SkuName), + "Insufficient quota for model %s %s"+ + " in %s\n"+ + "Requested: %.0f · Available: %.0f", + output.WithHighLightFormat( + "%q", r.dep.ModelName), + output.WithGrayFormat( + "(SKU: %s)", r.dep.SkuName), output.WithHighLightFormat(loc), totalRequired, remaining, diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index 2ac2d3200f3..b271e49b8b8 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -52,8 +52,7 @@ func (r *PreflightReport) ToString(currentIndentation string) string { if i > 0 { sb.WriteString("\n") } - sb.WriteString(fmt.Sprintf("%s%s %s", currentIndentation, warningPrefix, w.Message)) - writeItemSuggestion(&sb, currentIndentation, w) + writeItem(&sb, currentIndentation, warningPrefix, w) } if len(warnings) > 0 && len(errors) > 0 { @@ -64,28 +63,37 @@ func (r *PreflightReport) ToString(currentIndentation string) string { if i > 0 { sb.WriteString("\n") } - sb.WriteString(fmt.Sprintf("%s%s %s", currentIndentation, failedPrefix, e.Message)) - writeItemSuggestion(&sb, currentIndentation, e) + writeItem(&sb, currentIndentation, failedPrefix, e) } return sb.String() } -// writeItemSuggestion appends the suggestion and links for a report item, if present. -func writeItemSuggestion(sb *strings.Builder, indent string, item PreflightReportItem) { +// writeItem renders a single report item with proper indentation for multi-line +// messages, suggestions, and links. Continuation lines in the message are +// indented to align with the first line's text (after the prefix). +func writeItem( + sb *strings.Builder, indent string, prefix string, item PreflightReportItem, +) { + lines := strings.Split(item.Message, "\n") + sb.WriteString(fmt.Sprintf("%s%s %s", indent, prefix, lines[0])) + for _, line := range lines[1:] { + sb.WriteString(fmt.Sprintf("\n%s%s", indent, line)) + } + if item.Suggestion != "" { - sb.WriteString(fmt.Sprintf("\n%s %s %s", + sb.WriteString(fmt.Sprintf("\n%s%s %s", indent, output.WithHighLightFormat("Suggestion:"), item.Suggestion)) } for _, link := range item.Links { if link.Title != "" { - sb.WriteString(fmt.Sprintf("\n%s • %s", + sb.WriteString(fmt.Sprintf("\n%s• %s", indent, output.WithHyperlink(link.URL, link.Title))) } else { - sb.WriteString(fmt.Sprintf("\n%s • %s", + sb.WriteString(fmt.Sprintf("\n%s• %s", indent, output.WithLinkFormat(link.URL))) } diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index a3d489149a3..2c92d7ebdf0 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -5,6 +5,7 @@ package ux import ( "encoding/json" + "strings" "testing" "github.com/stretchr/testify/require" @@ -205,6 +206,52 @@ func TestPreflightReport_Indentation(t *testing.T) { require.Contains(t, result, "indented warning") } +func TestPreflightReport_MultiLineMessageIndentation(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + Message: "Model \"gpt-4o\" not found in eastus2\n" + + "Model not found in AI model catalog.", + }, + }, + } + + result := report.ToString(" ") + lines := strings.Split(result, "\n") + require.Len(t, lines, 2) + // First line has the warning prefix + require.Contains(t, lines[0], "(!) Warning:") + require.Contains(t, lines[0], "Model \"gpt-4o\" not found") + // Second line is indented at the same level + require.True(t, strings.HasPrefix(lines[1], " "), + "continuation line should be indented") + require.Contains(t, lines[1], "Model not found in AI model catalog.") +} + +func TestPreflightReport_MultiLineWithSuggestion(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + Message: "Insufficient quota for model \"gpt-4o\" in eastus2\n" + + "Requested: 99999 · Available: 140", + Suggestion: "Reduce capacity to 140.", + }, + }, + } + + result := report.ToString(" ") + // All three parts should be present in order + msgIdx := indexOf(result, "Insufficient quota") + detailIdx := indexOf(result, "Requested: 99999") + suggIdx := indexOf(result, "Reduce capacity") + require.Greater(t, detailIdx, msgIdx, + "detail should appear after title") + require.Greater(t, suggIdx, detailIdx, + "suggestion should appear after detail") +} + // indexOf returns the byte offset of substr in s, or -1 if not found. func indexOf(s, substr string) int { for i := range len(s) - len(substr) + 1 { diff --git a/cli/azd/test/functional/preflight_quota_test.go b/cli/azd/test/functional/preflight_quota_test.go index d0584467298..7c7763f07d4 100644 --- a/cli/azd/test/functional/preflight_quota_test.go +++ b/cli/azd/test/functional/preflight_quota_test.go @@ -54,7 +54,7 @@ func Test_CLI_PreflightQuota_RG_DefaultCapacity(t *testing.T) { // In this flow, declining the preflight warning is expected to return successfully, // and the output should contain the quota warning. output := result.Stdout + result.Stderr - require.Contains(t, output, "insufficient quota", + require.Contains(t, output, "Insufficient quota", "expected quota exceeded warning in output") } @@ -92,7 +92,7 @@ func Test_CLI_PreflightQuota_RG_InvalidModelName(t *testing.T) { ) require.NoError(t, err) output := result.Stdout + result.Stderr - require.Contains(t, output, "was not found in the AI model catalog", + require.Contains(t, output, "not found in AI model catalog", "expected model-not-found warning for invalid model name") require.Contains(t, output, "gpt-nonexistent-model") } @@ -131,7 +131,7 @@ func Test_CLI_PreflightQuota_RG_InvalidVersion(t *testing.T) { ) require.NoError(t, err) output := result.Stdout + result.Stderr - require.Contains(t, output, "was not found in the AI model catalog", + require.Contains(t, output, "not found in AI model catalog", "expected model-not-found warning for invalid version") } @@ -164,7 +164,7 @@ func Test_CLI_PreflightQuota_Sub_DefaultCapacity(t *testing.T) { ) require.NoError(t, err) output := result.Stdout + result.Stderr - require.Contains(t, output, "insufficient quota", + require.Contains(t, output, "Insufficient quota", "expected quota exceeded warning in output") } @@ -199,7 +199,7 @@ func Test_CLI_PreflightQuota_Sub_InvalidModelName(t *testing.T) { ) require.NoError(t, err) output := result.Stdout + result.Stderr - require.Contains(t, output, "was not found in the AI model catalog", + require.Contains(t, output, "not found in AI model catalog", "expected model-not-found warning for invalid model name") require.Contains(t, output, "gpt-555-turbo") } From 13397ea0af1364ec1953f8e48eb656f7e0e80772 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 5 May 2026 22:17:33 +0000 Subject: [PATCH 03/10] fix: address Copilot review feedback (iteration 1) - Fix writeItem doc comment to match actual indentation behavior - Strip ANSI escape sequences from Message/Suggestion in MarshalJSON - Use partition ordering (warnings-first) in MarshalJSON to match ToString - Add test for ANSI stripping and ordering verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/output/ux/preflight_report.go | 41 +++++++++++++++---- .../pkg/output/ux/preflight_report_test.go | 32 ++++++++++++++- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index b271e49b8b8..61346693f82 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -4,11 +4,14 @@ package ux import ( + "bytes" "encoding/json" "fmt" + "io" "strings" "github.com/azure/azure-dev/cli/azd/pkg/output" + "github.com/mattn/go-colorable" ) // PreflightReportItem represents a single finding from preflight validation. @@ -69,9 +72,9 @@ func (r *PreflightReport) ToString(currentIndentation string) string { return sb.String() } -// writeItem renders a single report item with proper indentation for multi-line -// messages, suggestions, and links. Continuation lines in the message are -// indented to align with the first line's text (after the prefix). +// writeItem renders a single report item with multi-line support. +// The first line is prefixed with the status indicator (e.g. "(!) Warning:"). +// Continuation lines in the message are indented at the same level as the prefix. func writeItem( sb *strings.Builder, indent string, prefix string, item PreflightReportItem, ) { @@ -115,8 +118,15 @@ func (r *PreflightReport) MarshalJSON() ([]byte, error) { Links []jsonLink `json:"links,omitempty"` } - items := make([]jsonItem, 0, len(r.Items)) - for _, item := range r.Items { + // Use partition ordering (warnings first, then errors) + // to match ToString() output order. + ordered := make( + []PreflightReportItem, 0, len(warnings)+len(errors)) + ordered = append(ordered, warnings...) + ordered = append(ordered, errors...) + + items := make([]jsonItem, 0, len(ordered)) + for _, item := range ordered { severity := "warning" if item.IsError { severity = "error" @@ -124,8 +134,8 @@ func (r *PreflightReport) MarshalJSON() ([]byte, error) { ji := jsonItem{ Severity: severity, DiagnosticID: item.DiagnosticID, - Message: item.Message, - Suggestion: item.Suggestion, + Message: stripAnsi(item.Message), + Suggestion: stripAnsi(item.Suggestion), } for _, link := range item.Links { ji.Links = append(ji.Links, jsonLink(link)) @@ -178,3 +188,20 @@ func (r *PreflightReport) partition() (warnings, errors []PreflightReportItem) { } return warnings, errors } + +// stripAnsi removes ANSI escape sequences from a string for +// machine-readable output (e.g. JSON). +func stripAnsi(s string) string { + if s == "" { + return s + } + var buf bytes.Buffer + // colorable.NewNonColorable strips ANSI sequences. + if _, err := io.Copy( + colorable.NewNonColorable(&buf), + strings.NewReader(s), + ); err != nil { + return s + } + return buf.String() +} diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index 2c92d7ebdf0..81138b052e2 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -142,7 +142,10 @@ func TestPreflightReport_NoSuggestion(t *testing.T) { func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { report := &PreflightReport{ + // Insert error first, then warning — JSON should reorder + // to warnings-first (matching ToString behavior). Items: []PreflightReportItem{ + {IsError: true, DiagnosticID: "role_error", Message: "role missing"}, { IsError: false, DiagnosticID: "ai_model_quota_exceeded", @@ -152,7 +155,6 @@ func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { {URL: "https://example.com/quotas", Title: "Quota docs"}, }, }, - {IsError: true, DiagnosticID: "role_error", Message: "role missing"}, }, } @@ -180,7 +182,7 @@ func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { require.Contains(t, parsed.Summary, "1 error(s)") require.Len(t, parsed.Items, 2) - // First item: warning with suggestion and links + // First item should be warning (partition ordering) require.Equal(t, "warning", parsed.Items[0].Severity) require.Equal(t, "ai_model_quota_exceeded", parsed.Items[0].DiagnosticID) require.Equal(t, "Reduce capacity to 140.", parsed.Items[0].Suggestion) @@ -194,6 +196,32 @@ func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { require.Empty(t, parsed.Items[1].Links) } +func TestPreflightReport_MarshalJSON_StripsAnsi(t *testing.T) { + // Simulate ANSI-formatted message (like output.WithHighLightFormat) + ansiMsg := "\x1b[36mmodel\x1b[0m not found in \x1b[36meastus2\x1b[0m" + ansiSugg := "Run \x1b[36mazd env set AZURE_LOCATION\x1b[0m" + + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + Message: ansiMsg, + Suggestion: ansiSugg, + }, + }, + } + + data, err := json.Marshal(report) + require.NoError(t, err) + + raw := string(data) + // JSON output should not contain ANSI escape sequences + require.NotContains(t, raw, "\x1b[", + "JSON output should not contain ANSI escapes") + require.Contains(t, raw, "model not found in eastus2") + require.Contains(t, raw, "Run azd env set AZURE_LOCATION") +} + func TestPreflightReport_Indentation(t *testing.T) { report := &PreflightReport{ Items: []PreflightReportItem{ From 0f5989b189ed3f7da4065d73ff255aa6e2f743cd Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 02:24:56 +0000 Subject: [PATCH 04/10] Add snapshot tests for preflight warning rendering Add 6 snapshot tests covering each warning diagnostic type individually plus one combined test showing all warnings together: - role_assignment_missing (with suggestion) - role_assignment_conditional (no suggestion) - reserved_resource_name (with links, no suggestion) - ai_model_not_found (with suggestion and links) - ai_model_quota_exceeded (with suggestion and links) - all warnings combined Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pkg/output/ux/preflight_report_test.go | 181 ++++++++++++++++++ ...flightReport_Snapshot_AiModelNotFound.snap | 4 + ...tReport_Snapshot_AiModelQuotaExceeded.snap | 4 + ...htReport_Snapshot_AllWarningsCombined.snap | 11 ++ ...tReport_Snapshot_ReservedResourceName.snap | 3 + ...rt_Snapshot_RoleAssignmentConditional.snap | 2 + ...Report_Snapshot_RoleAssignmentMissing.snap | 3 + 7 files changed, 208 insertions(+) create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelNotFound.snap create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelQuotaExceeded.snap create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AllWarningsCombined.snap create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_ReservedResourceName.snap create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentConditional.snap create mode 100644 cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentMissing.snap diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index 81138b052e2..221fb2c6d9c 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/azure/azure-dev/cli/azd/test/snapshot" "github.com/stretchr/testify/require" ) @@ -289,3 +290,183 @@ func indexOf(s, substr string) int { } return -1 } + +// Snapshot tests — one per diagnostic type, plus one combined report. +// Update snapshots with: UPDATE_SNAPSHOTS=true go test ./pkg/output/ux/... + +func TestPreflightReport_Snapshot_RoleAssignmentMissing(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "role_assignment_missing", + Message: "Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb)" + + " lacks role assignment permissions on" + + " subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a\n" + + "The deployment includes role assignments" + + " and will fail without" + + " Microsoft.Authorization/roleAssignments/write" + + " permission.", + Suggestion: "Ensure you have the" + + " 'Role Based Access Control Administrator'," + + " 'User Access Administrator'," + + " 'Owner', or a custom role with" + + " 'Microsoft.Authorization/roleAssignments/write'" + + " assigned to your account.", + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} + +func TestPreflightReport_Snapshot_RoleAssignmentConditional(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "role_assignment_conditional", + Message: "Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb)" + + " has conditional role assignment permissions on" + + " subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a\n" + + "An ABAC condition may restrict which roles can be assigned." + + " The deployment may fail if the condition does not permit" + + " the specific role assignments in the template.", + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} + +func TestPreflightReport_Snapshot_ReservedResourceName(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "reserved_resource_name", + Message: "Resource \"login-server\"" + + " (Microsoft.Web/sites)" + + " contains the reserved word \"login\"\n" + + "Azure does not allow reserved words in" + + " resource names. The deployment will fail.", + Links: []PreflightReportLink{ + { + URL: "https://learn.microsoft.com/azure/" + + "azure-resource-manager/templates/" + + "error-reserved-resource-name", + Title: "Reserved resource name errors", + }, + }, + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} + +func TestPreflightReport_Snapshot_AiModelNotFound(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "ai_model_not_found", + Message: "Model \"no-model\" (SKU: GlobalStandard)" + + " not found in eastus2\n" + + "Model not found in AI model catalog." + + " Provisioning will likely fail.", + Suggestion: "Verify the model name, SKU," + + " and version are correct.", + Links: []PreflightReportLink{ + { + URL: "https://learn.microsoft.com/azure/ai-services/openai/concepts/models", + Title: "Azure OpenAI supported models and regions", + }, + }, + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} + +func TestPreflightReport_Snapshot_AiModelQuotaExceeded(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "ai_model_quota_exceeded", + Message: "Insufficient quota for model \"gpt-4o\"" + + " (SKU: GlobalStandard) in eastus2\n" + + "Requested: 99999 · Available: 140", + Suggestion: "Reduce the requested capacity to 140" + + " or change your deployment location via" + + " azd env set AZURE_LOCATION ." + + " You can also request a quota increase" + + " in the Azure portal.", + Links: []PreflightReportLink{ + { + URL: "https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal", + Title: "Increase Azure subscription quotas", + }, + }, + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} + +func TestPreflightReport_Snapshot_AllWarningsCombined(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{ + { + IsError: false, + DiagnosticID: "role_assignment_missing", + Message: "Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb)" + + " lacks role assignment permissions on" + + " subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a\n" + + "The deployment includes role assignments" + + " and will fail without" + + " Microsoft.Authorization/roleAssignments/write" + + " permission.", + Suggestion: "Ensure you have the" + + " 'Role Based Access Control Administrator'," + + " 'User Access Administrator'," + + " 'Owner', or a custom role with" + + " 'Microsoft.Authorization/roleAssignments/write'" + + " assigned to your account.", + }, + { + IsError: false, + DiagnosticID: "ai_model_not_found", + Message: "Model \"no-model\" (SKU: GlobalStandard)" + + " not found in eastus2\n" + + "Model not found in AI model catalog." + + " Provisioning will likely fail.", + Suggestion: "Verify the model name, SKU," + + " and version are correct.", + Links: []PreflightReportLink{ + { + URL: "https://learn.microsoft.com/azure/ai-services/openai/concepts/models", + Title: "Azure OpenAI supported models and regions", + }, + }, + }, + { + IsError: false, + DiagnosticID: "ai_model_quota_exceeded", + Message: "Insufficient quota for model \"gpt-4o\"" + + " (SKU: GlobalStandard) in eastus2\n" + + "Requested: 99999 · Available: 140", + Suggestion: "Reduce the requested capacity to 140" + + " or change your deployment location via" + + " azd env set AZURE_LOCATION ." + + " You can also request a quota increase" + + " in the Azure portal.", + Links: []PreflightReportLink{ + { + URL: "https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal", + Title: "Increase Azure subscription quotas", + }, + }, + }, + }, + } + snapshot.SnapshotT(t, report.ToString(" ")) +} diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelNotFound.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelNotFound.snap new file mode 100644 index 00000000000..aa296239684 --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelNotFound.snap @@ -0,0 +1,4 @@ + (!) Warning: Model "no-model" (SKU: GlobalStandard) not found in eastus2 + Model not found in AI model catalog. Provisioning will likely fail. + Suggestion: Verify the model name, SKU, and version are correct. + • https://learn.microsoft.com/azure/ai-services/openai/concepts/models diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelQuotaExceeded.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelQuotaExceeded.snap new file mode 100644 index 00000000000..808765abdf8 --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AiModelQuotaExceeded.snap @@ -0,0 +1,4 @@ + (!) Warning: Insufficient quota for model "gpt-4o" (SKU: GlobalStandard) in eastus2 + Requested: 99999 · Available: 140 + Suggestion: Reduce the requested capacity to 140 or change your deployment location via azd env set AZURE_LOCATION . You can also request a quota increase in the Azure portal. + • https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AllWarningsCombined.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AllWarningsCombined.snap new file mode 100644 index 00000000000..e7ccd886b0c --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_AllWarningsCombined.snap @@ -0,0 +1,11 @@ + (!) Warning: Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb) lacks role assignment permissions on subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a + The deployment includes role assignments and will fail without Microsoft.Authorization/roleAssignments/write permission. + Suggestion: Ensure you have the 'Role Based Access Control Administrator', 'User Access Administrator', 'Owner', or a custom role with 'Microsoft.Authorization/roleAssignments/write' assigned to your account. + (!) Warning: Model "no-model" (SKU: GlobalStandard) not found in eastus2 + Model not found in AI model catalog. Provisioning will likely fail. + Suggestion: Verify the model name, SKU, and version are correct. + • https://learn.microsoft.com/azure/ai-services/openai/concepts/models + (!) Warning: Insufficient quota for model "gpt-4o" (SKU: GlobalStandard) in eastus2 + Requested: 99999 · Available: 140 + Suggestion: Reduce the requested capacity to 140 or change your deployment location via azd env set AZURE_LOCATION . You can also request a quota increase in the Azure portal. + • https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_ReservedResourceName.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_ReservedResourceName.snap new file mode 100644 index 00000000000..9404f8a8761 --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_ReservedResourceName.snap @@ -0,0 +1,3 @@ + (!) Warning: Resource "login-server" (Microsoft.Web/sites) contains the reserved word "login" + Azure does not allow reserved words in resource names. The deployment will fail. + • https://learn.microsoft.com/azure/azure-resource-manager/templates/error-reserved-resource-name diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentConditional.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentConditional.snap new file mode 100644 index 00000000000..8a7fbec1b12 --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentConditional.snap @@ -0,0 +1,2 @@ + (!) Warning: Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb) has conditional role assignment permissions on subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a + An ABAC condition may restrict which roles can be assigned. The deployment may fail if the condition does not permit the specific role assignments in the template. diff --git a/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentMissing.snap b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentMissing.snap new file mode 100644 index 00000000000..eee09419459 --- /dev/null +++ b/cli/azd/pkg/output/ux/testdata/TestPreflightReport_Snapshot_RoleAssignmentMissing.snap @@ -0,0 +1,3 @@ + (!) Warning: Principal (5a3acce7-bcc4-4ebc-b4b3-c3b9f17535cb) lacks role assignment permissions on subscription 3819cb9d-0f7c-4284-9e93-220e7fb2367a + The deployment includes role assignments and will fail without Microsoft.Authorization/roleAssignments/write permission. + Suggestion: Ensure you have the 'Role Based Access Control Administrator', 'User Access Administrator', 'Owner', or a custom role with 'Microsoft.Authorization/roleAssignments/write' assigned to your account. From 0d066d7ece04322ce11cafa9e7ff3f20c55ed1a7 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 02:45:15 +0000 Subject: [PATCH 05/10] fix: address Copilot review feedback (iteration 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix import ordering: stdlib → external → internal - Revert MarshalJSON to use EventEnvelope via EventForMessage for consistency with the --output json JSONL event stream - Preallocate links slice in validatePreflight Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../provisioning/bicep/bicep_provider.go | 9 ++- cli/azd/pkg/output/ux/preflight_report.go | 72 +---------------- .../pkg/output/ux/preflight_report_test.go | 80 +++---------------- 3 files changed, 19 insertions(+), 142 deletions(-) diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index db00dc2f609..e76bfd56231 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -2636,12 +2636,13 @@ func (p *BicepProvider) validatePreflight( if len(results) > 0 { report := &ux.PreflightReport{} for _, result := range results { - var links []ux.PreflightReportLink - for _, l := range result.Links { - links = append(links, ux.PreflightReportLink{ + links := make( + []ux.PreflightReportLink, len(result.Links)) + for i, l := range result.Links { + links[i] = ux.PreflightReportLink{ URL: l.URL, Title: l.Title, - }) + } } report.Items = append(report.Items, ux.PreflightReportItem{ IsError: result.Severity == PreflightCheckError, diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index 61346693f82..395d21626c6 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -4,14 +4,11 @@ package ux import ( - "bytes" "encoding/json" "fmt" - "io" "strings" "github.com/azure/azure-dev/cli/azd/pkg/output" - "github.com/mattn/go-colorable" ) // PreflightReportItem represents a single finding from preflight validation. @@ -106,55 +103,9 @@ func writeItem( func (r *PreflightReport) MarshalJSON() ([]byte, error) { warnings, errors := r.partition() - type jsonLink struct { - URL string `json:"url"` - Title string `json:"title,omitempty"` - } - type jsonItem struct { - Severity string `json:"severity"` - DiagnosticID string `json:"diagnosticId,omitempty"` - Message string `json:"message"` - Suggestion string `json:"suggestion,omitempty"` - Links []jsonLink `json:"links,omitempty"` - } - - // Use partition ordering (warnings first, then errors) - // to match ToString() output order. - ordered := make( - []PreflightReportItem, 0, len(warnings)+len(errors)) - ordered = append(ordered, warnings...) - ordered = append(ordered, errors...) - - items := make([]jsonItem, 0, len(ordered)) - for _, item := range ordered { - severity := "warning" - if item.IsError { - severity = "error" - } - ji := jsonItem{ - Severity: severity, - DiagnosticID: item.DiagnosticID, - Message: stripAnsi(item.Message), - Suggestion: stripAnsi(item.Suggestion), - } - for _, link := range item.Links { - ji.Links = append(ji.Links, jsonLink(link)) - } - items = append(items, ji) - } - - result := struct { - Type string `json:"type"` - Summary string `json:"summary"` - Items []jsonItem `json:"items"` - }{ - Type: "preflight", - Summary: fmt.Sprintf("preflight: %d warning(s), %d error(s)", - len(warnings), len(errors)), - Items: items, - } - - return json.Marshal(result) + return json.Marshal(output.EventForMessage( + fmt.Sprintf("preflight: %d warning(s), %d error(s)", + len(warnings), len(errors)))) } // HasErrors returns true if the report contains at least one error-level item. @@ -188,20 +139,3 @@ func (r *PreflightReport) partition() (warnings, errors []PreflightReportItem) { } return warnings, errors } - -// stripAnsi removes ANSI escape sequences from a string for -// machine-readable output (e.g. JSON). -func stripAnsi(s string) string { - if s == "" { - return s - } - var buf bytes.Buffer - // colorable.NewNonColorable strips ANSI sequences. - if _, err := io.Copy( - colorable.NewNonColorable(&buf), - strings.NewReader(s), - ); err != nil { - return s - } - return buf.String() -} diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index 221fb2c6d9c..f7c122401c9 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -141,86 +141,28 @@ func TestPreflightReport_NoSuggestion(t *testing.T) { require.NotContains(t, result, "Suggestion:") } -func TestPreflightReport_MarshalJSON_WithSuggestions(t *testing.T) { +func TestPreflightReport_MarshalJSON_Envelope(t *testing.T) { report := &PreflightReport{ - // Insert error first, then warning — JSON should reorder - // to warnings-first (matching ToString behavior). Items: []PreflightReportItem{ - {IsError: true, DiagnosticID: "role_error", Message: "role missing"}, - { - IsError: false, - DiagnosticID: "ai_model_quota_exceeded", - Message: "insufficient quota", - Suggestion: "Reduce capacity to 140.", - Links: []PreflightReportLink{ - {URL: "https://example.com/quotas", Title: "Quota docs"}, - }, - }, + {IsError: false, Message: "w1", Suggestion: "fix it"}, + {IsError: true, Message: "e1"}, }, } data, err := json.Marshal(report) require.NoError(t, err) + // MarshalJSON wraps output in EventEnvelope var parsed struct { - Type string `json:"type"` - Summary string `json:"summary"` - Items []struct { - Severity string `json:"severity"` - DiagnosticID string `json:"diagnosticId"` - Message string `json:"message"` - Suggestion string `json:"suggestion"` - Links []struct { - URL string `json:"url"` - Title string `json:"title"` - } `json:"links"` - } `json:"items"` + Type string `json:"type"` + Data struct { + Message string `json:"message"` + } `json:"data"` } require.NoError(t, json.Unmarshal(data, &parsed)) - - require.Equal(t, "preflight", parsed.Type) - require.Contains(t, parsed.Summary, "1 warning(s)") - require.Contains(t, parsed.Summary, "1 error(s)") - require.Len(t, parsed.Items, 2) - - // First item should be warning (partition ordering) - require.Equal(t, "warning", parsed.Items[0].Severity) - require.Equal(t, "ai_model_quota_exceeded", parsed.Items[0].DiagnosticID) - require.Equal(t, "Reduce capacity to 140.", parsed.Items[0].Suggestion) - require.Len(t, parsed.Items[0].Links, 1) - require.Equal(t, "https://example.com/quotas", parsed.Items[0].Links[0].URL) - require.Equal(t, "Quota docs", parsed.Items[0].Links[0].Title) - - // Second item: error without suggestion - require.Equal(t, "error", parsed.Items[1].Severity) - require.Empty(t, parsed.Items[1].Suggestion) - require.Empty(t, parsed.Items[1].Links) -} - -func TestPreflightReport_MarshalJSON_StripsAnsi(t *testing.T) { - // Simulate ANSI-formatted message (like output.WithHighLightFormat) - ansiMsg := "\x1b[36mmodel\x1b[0m not found in \x1b[36meastus2\x1b[0m" - ansiSugg := "Run \x1b[36mazd env set AZURE_LOCATION\x1b[0m" - - report := &PreflightReport{ - Items: []PreflightReportItem{ - { - IsError: false, - Message: ansiMsg, - Suggestion: ansiSugg, - }, - }, - } - - data, err := json.Marshal(report) - require.NoError(t, err) - - raw := string(data) - // JSON output should not contain ANSI escape sequences - require.NotContains(t, raw, "\x1b[", - "JSON output should not contain ANSI escapes") - require.Contains(t, raw, "model not found in eastus2") - require.Contains(t, raw, "Run azd env set AZURE_LOCATION") + require.Equal(t, "consoleMessage", string(parsed.Type)) + require.Contains(t, parsed.Data.Message, "1 warning(s)") + require.Contains(t, parsed.Data.Message, "1 error(s)") } func TestPreflightReport_Indentation(t *testing.T) { From 82d6d4bfd4c7cc164d4f2d592292468876fb0434 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 02:51:12 +0000 Subject: [PATCH 06/10] fix: address Copilot review feedback (iteration 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix import ordering in test file: stdlib → external → internal - Update PR description to match actual MarshalJSON behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/output/ux/preflight_report_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index f7c122401c9..db041556497 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -8,8 +8,9 @@ import ( "strings" "testing" - "github.com/azure/azure-dev/cli/azd/test/snapshot" "github.com/stretchr/testify/require" + + "github.com/azure/azure-dev/cli/azd/test/snapshot" ) func TestPreflightReport_EmptyItems(t *testing.T) { From eff4ac85b8dc892f542b15d4611ff62933c7d467 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 02:58:38 +0000 Subject: [PATCH 07/10] fix: address Copilot review feedback (iteration 5) - Update PreflightReportLink.Title doc comment to note terminal-only behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/output/ux/preflight_report.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index 395d21626c6..22bbe76dd72 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -30,7 +30,8 @@ type PreflightReportItem struct { type PreflightReportLink struct { // URL is the link target. URL string - // Title is the display text (optional — if empty, the URL is shown). + // Title is the display text for terminal hyperlinks (optional). + // In non-terminal output the URL is shown regardless of Title. Title string } From 46af43da0d86688345377852cf3d68a8bf4cd785 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 03:06:36 +0000 Subject: [PATCH 08/10] fix: address Copilot review feedback (iteration 6) - Guard writeItem against empty message to prevent rendering empty prefix - Align PreflightCheckLink.Title doc comment with UX counterpart Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/infra/provisioning/bicep/local_preflight.go | 3 ++- cli/azd/pkg/output/ux/preflight_report.go | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go index 9f4471e98fe..ce6526677c1 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go +++ b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go @@ -282,7 +282,8 @@ type PreflightCheckResult struct { type PreflightCheckLink struct { // URL is the link target. URL string - // Title is the display text (optional — if empty, the URL is shown). + // Title is the display text for terminal hyperlinks (optional). + // In non-terminal output the URL is shown regardless of Title. Title string } diff --git a/cli/azd/pkg/output/ux/preflight_report.go b/cli/azd/pkg/output/ux/preflight_report.go index 22bbe76dd72..250b74e7ff9 100644 --- a/cli/azd/pkg/output/ux/preflight_report.go +++ b/cli/azd/pkg/output/ux/preflight_report.go @@ -76,6 +76,9 @@ func (r *PreflightReport) ToString(currentIndentation string) string { func writeItem( sb *strings.Builder, indent string, prefix string, item PreflightReportItem, ) { + if item.Message == "" { + return + } lines := strings.Split(item.Message, "\n") sb.WriteString(fmt.Sprintf("%s%s %s", indent, prefix, lines[0])) for _, line := range lines[1:] { From 8ab8660594041a820123aa5dfcbd26d060f415ed Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 03:21:36 +0000 Subject: [PATCH 09/10] fix: address Copilot review feedback (iteration 7) - Handle zero remaining quota in suggestion text: show 'No quota is available' instead of misleading 'Reduce capacity to 0' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../provisioning/bicep/bicep_provider.go | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index e76bfd56231..9ce3fdab8c4 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -3011,14 +3011,31 @@ func (p *BicepProvider) checkAiModelQuota( if remaining < totalRequired { reportedUsage[r.usageName] = true - suggestion := fmt.Sprintf( - "Reduce the requested capacity to %.0f"+ - " or change your deployment location via %s."+ - " You can also request a quota increase in the Azure portal.", - remaining, - output.WithHighLightFormat( - "azd env set AZURE_LOCATION "), - ) + var suggestion string + if remaining > 0 { + suggestion = fmt.Sprintf( + "Reduce the requested capacity"+ + " to %.0f or change your"+ + " deployment location via %s."+ + " You can also request a quota"+ + " increase in the Azure portal.", + remaining, + output.WithHighLightFormat( + "azd env set"+ + " AZURE_LOCATION "), + ) + } else { + suggestion = fmt.Sprintf( + "No quota is available."+ + " Change your deployment"+ + " location via %s or request"+ + " a quota increase in the"+ + " Azure portal.", + output.WithHighLightFormat( + "azd env set"+ + " AZURE_LOCATION "), + ) + } results = append(results, PreflightCheckResult{ Severity: PreflightCheckWarning, From 4d3193598c6e2166a89cb69b3631d7b55b161ef4 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 7 May 2026 20:35:22 +0000 Subject: [PATCH 10/10] Address human review feedback - Add Suggestion: assertion to functional tests (wbreza #1) - Consolidate PreflightCheckLink into ux.PreflightReportLink, eliminating duplicate type and manual conversion (wbreza #2) - Add 7 table-driven edge case tests for writeItem (wbreza #5) - Fix PR description Before block (wbreza #7) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/magefile.go | 16 ++-- .../provisioning/bicep/bicep_provider.go | 16 +--- .../provisioning/bicep/local_preflight.go | 12 +-- .../pkg/output/ux/preflight_report_test.go | 78 +++++++++++++++++++ .../test/functional/preflight_quota_test.go | 2 + 5 files changed, 94 insertions(+), 30 deletions(-) diff --git a/cli/azd/magefile.go b/cli/azd/magefile.go index 2ff84a0b31e..f56d0f5048a 100644 --- a/cli/azd/magefile.go +++ b/cli/azd/magefile.go @@ -566,15 +566,15 @@ var excludedPlaybackTests = map[string]string{ // Recordings affected by feat/exegraph: the graph-driven up/provision path // introduces legitimate new HTTP interactions (layer hash probes, resource-group // existence checks). Must be re-recorded with live Azure credentials before merge. - "Test_DeploymentStacks": "needs re-record for feat/exegraph graph-driven provision", - "Test_CLI_ProvisionState": "needs re-record for feat/exegraph graph-driven provision", - "Test_CLI_InfraCreateAndDeleteUpperCase": "needs re-record for feat/exegraph graph-driven provision", - "Test_CLI_PreflightQuota_Sub_DefaultCapacity": "stale recording; missing extension registry + resource group interactions", - "Test_CLI_PreflightQuota_Sub_InvalidModelName": "stale recording; missing extension registry + resource group interactions", + "Test_DeploymentStacks": "needs re-record for feat/exegraph graph-driven provision", + "Test_CLI_ProvisionState": "needs re-record for feat/exegraph graph-driven provision", + "Test_CLI_InfraCreateAndDeleteUpperCase": "needs re-record for feat/exegraph graph-driven provision", + "Test_CLI_PreflightQuota_Sub_DefaultCapacity": "stale recording; missing extension registry + resource group interactions", + "Test_CLI_PreflightQuota_Sub_InvalidModelName": "stale recording; missing extension registry + resource group interactions", "Test_CLI_PreflightQuota_Sub_DifferentLocation": "stale recording; missing extension registry + resource group interactions", - "Test_CLI_PreflightQuota_RG_DefaultCapacity": "stale recording; missing extension registry + resource group interactions", - "Test_CLI_PreflightQuota_RG_InvalidVersion": "stale recording; missing extension registry + resource group interactions", - "Test_CLI_PreflightQuota_RG_InvalidModelName": "stale recording; missing extension registry + resource group interactions", + "Test_CLI_PreflightQuota_RG_DefaultCapacity": "stale recording; missing extension registry + resource group interactions", + "Test_CLI_PreflightQuota_RG_InvalidVersion": "stale recording; missing extension registry + resource group interactions", + "Test_CLI_PreflightQuota_RG_InvalidModelName": "stale recording; missing extension registry + resource group interactions", } // discoverPlaybackTests scans the recordings directory for .yaml files and diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index 9ce3fdab8c4..0327f07a8f9 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -2636,20 +2636,12 @@ func (p *BicepProvider) validatePreflight( if len(results) > 0 { report := &ux.PreflightReport{} for _, result := range results { - links := make( - []ux.PreflightReportLink, len(result.Links)) - for i, l := range result.Links { - links[i] = ux.PreflightReportLink{ - URL: l.URL, - Title: l.Title, - } - } report.Items = append(report.Items, ux.PreflightReportItem{ IsError: result.Severity == PreflightCheckError, DiagnosticID: result.DiagnosticID, Message: result.Message, Suggestion: result.Suggestion, - Links: links, + Links: result.Links, }) } p.console.MessageUxItem(ctx, report) @@ -2843,7 +2835,7 @@ func (p *BicepProvider) checkReservedResourceNames( resourceName, resourceType, v.matchType, v.reservedWord, ), - Links: []PreflightCheckLink{ + Links: []ux.PreflightReportLink{ { URL: docsLink, Title: "Reserved resource name errors", @@ -2967,7 +2959,7 @@ func (p *BicepProvider) checkAiModelQuota( ), Suggestion: "Verify the model name, SKU," + " and version are correct.", - Links: []PreflightCheckLink{ + Links: []ux.PreflightReportLink{ { URL: "https://learn.microsoft.com/" + "azure/ai-services/openai/" + @@ -3053,7 +3045,7 @@ func (p *BicepProvider) checkAiModelQuota( remaining, ), Suggestion: suggestion, - Links: []PreflightCheckLink{ + Links: []ux.PreflightReportLink{ { URL: "https://learn.microsoft.com/azure/quotas/quickstart-increase-quota-portal", Title: "Increase Azure subscription quotas", diff --git a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go index ce6526677c1..1c61a00c9ac 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go +++ b/cli/azd/pkg/infra/provisioning/bicep/local_preflight.go @@ -18,6 +18,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azure" "github.com/azure/azure-dev/cli/azd/pkg/infra" "github.com/azure/azure-dev/cli/azd/pkg/input" + "github.com/azure/azure-dev/cli/azd/pkg/output/ux" "github.com/azure/azure-dev/cli/azd/pkg/tools/bicep" ) @@ -275,16 +276,7 @@ type PreflightCheckResult struct { // It should be dynamically generated with context-specific advice when possible. Suggestion string // Links is an optional list of reference links related to the finding. - Links []PreflightCheckLink -} - -// PreflightCheckLink represents a reference link attached to a preflight check result. -type PreflightCheckLink struct { - // URL is the link target. - URL string - // Title is the display text for terminal hyperlinks (optional). - // In non-terminal output the URL is shown regardless of Title. - Title string + Links []ux.PreflightReportLink } // validationContext provides the data and utilities available to preflight check functions. diff --git a/cli/azd/pkg/output/ux/preflight_report_test.go b/cli/azd/pkg/output/ux/preflight_report_test.go index db041556497..e0d0e36d590 100644 --- a/cli/azd/pkg/output/ux/preflight_report_test.go +++ b/cli/azd/pkg/output/ux/preflight_report_test.go @@ -234,6 +234,84 @@ func indexOf(s, substr string) int { return -1 } +func TestPreflightReport_WriteItem_EdgeCases(t *testing.T) { + tests := []struct { + name string + item PreflightReportItem + contains []string + excludes []string + }{ + { + name: "empty message", + item: PreflightReportItem{Message: ""}, + excludes: []string{"Warning"}, + }, + { + name: "trailing newline in message", + item: PreflightReportItem{ + Message: "title line\n", + }, + contains: []string{"title line"}, + }, + { + name: "consecutive newlines in message", + item: PreflightReportItem{ + Message: "first\n\nthird", + }, + contains: []string{"first", "third"}, + }, + { + name: "nil links slice", + item: PreflightReportItem{ + Message: "msg", + Links: nil, + }, + contains: []string{"msg"}, + excludes: []string{"•"}, + }, + { + name: "empty links slice", + item: PreflightReportItem{ + Message: "msg", + Links: []PreflightReportLink{}, + }, + contains: []string{"msg"}, + excludes: []string{"•"}, + }, + { + name: "empty suggestion string", + item: PreflightReportItem{ + Message: "msg", + Suggestion: "", + }, + contains: []string{"msg"}, + excludes: []string{"Suggestion:"}, + }, + { + name: "message with leading newline", + item: PreflightReportItem{ + Message: "\nleading newline", + }, + contains: []string{"leading newline"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + report := &PreflightReport{ + Items: []PreflightReportItem{tt.item}, + } + result := report.ToString(" ") + for _, s := range tt.contains { + require.Contains(t, result, s) + } + for _, s := range tt.excludes { + require.NotContains(t, result, s) + } + }) + } +} + // Snapshot tests — one per diagnostic type, plus one combined report. // Update snapshots with: UPDATE_SNAPSHOTS=true go test ./pkg/output/ux/... diff --git a/cli/azd/test/functional/preflight_quota_test.go b/cli/azd/test/functional/preflight_quota_test.go index 7c7763f07d4..75be7bebd49 100644 --- a/cli/azd/test/functional/preflight_quota_test.go +++ b/cli/azd/test/functional/preflight_quota_test.go @@ -56,6 +56,8 @@ func Test_CLI_PreflightQuota_RG_DefaultCapacity(t *testing.T) { output := result.Stdout + result.Stderr require.Contains(t, output, "Insufficient quota", "expected quota exceeded warning in output") + require.Contains(t, output, "Suggestion:", + "expected actionable suggestion in output") } // Test_CLI_PreflightQuota_RG_InvalidModelName verifies a warning when the model name